Skip to content

Commit 5cff4e5

Browse files
pedroerpfacebook-github-bot
pedroerp
authored and
facebook-github-bot
committed
Remove old time zone ID based API from Timestamp (facebookincubator#10632)
Summary: Pull Request resolved: facebookincubator#10632 With the recent refactor, all conversions can now be done through the TimeZone pointer, so there is not need to do multiple hops of conversion from ID to name, from name to time zone object. Removing the legacy APIs and cleaning up callsites. Reviewed By: kagamiori Differential Revision: D60477527 fbshipit-source-id: fb026efb3d951aa54b370c385b79f2395276856c
1 parent 484f78c commit 5cff4e5

13 files changed

+165
-198
lines changed

‎velox/expression/PrestoCastHooks.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ Expected<Timestamp> PrestoCastHooks::castStringToTimestamp(
5454
// If the parsed string has timezone information, convert the timestamp at
5555
// GMT at that time. For example, "1970-01-01 00:00:00 -00:01" is 60 seconds
5656
// at GMT.
57-
if (result.second != -1) {
58-
result.first.toGMT(result.second);
57+
if (result.second != nullptr) {
58+
result.first.toGMT(*result.second);
5959

6060
}
6161
// If no timezone information is available in the input string, check if we

‎velox/functions/prestosql/DateTimeFunctions.h

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,8 @@ struct TimestampWithTimezoneSupport {
116116
bool asGMT = false) {
117117
auto timestamp = unpackTimestampUtc(timestampWithTimezone);
118118
if (!asGMT) {
119-
timestamp.toTimezone(unpackZoneKeyId(timestampWithTimezone));
119+
timestamp.toTimezone(
120+
*tz::locateZone(unpackZoneKeyId(timestampWithTimezone)));
120121
}
121122

122123
return timestamp;
@@ -129,7 +130,7 @@ struct TimestampWithTimezoneSupport {
129130
Timestamp inputTimeStamp = this->toTimestamp(timestampWithTimezone);
130131
// Create a copy of inputTimeStamp and convert it to GMT
131132
auto gmtTimeStamp = inputTimeStamp;
132-
gmtTimeStamp.toGMT(unpackZoneKeyId(timestampWithTimezone));
133+
gmtTimeStamp.toGMT(*tz::locateZone(unpackZoneKeyId(timestampWithTimezone)));
133134

134135
// Get offset in seconds with GMT and convert to hour
135136
return (inputTimeStamp.getSeconds() - gmtTimeStamp.getSeconds());
@@ -1166,7 +1167,7 @@ struct DateTruncFunction : public TimestampWithTimezoneSupport<T> {
11661167
adjustDateTime(dateTime, unit);
11671168
timestamp =
11681169
Timestamp::fromMillis(Timestamp::calendarUtcToEpoch(dateTime) * 1000);
1169-
timestamp.toGMT(unpackZoneKeyId(timestampWithTimezone));
1170+
timestamp.toGMT(*tz::locateZone(unpackZoneKeyId(timestampWithTimezone)));
11701171

11711172
result = pack(timestamp, unpackZoneKeyId(timestampWithTimezone));
11721173
}
@@ -1472,7 +1473,7 @@ struct FromIso8601Timestamp {
14721473
const arg_type<Varchar>* /*input*/) {
14731474
auto sessionTzName = config.sessionTimezone();
14741475
if (!sessionTzName.empty()) {
1475-
sessionTzID_ = tz::getTimeZoneID(sessionTzName);
1476+
sessionTimeZone_ = tz::locateZone(sessionTzName);
14761477
}
14771478
}
14781479

@@ -1485,27 +1486,29 @@ struct FromIso8601Timestamp {
14851486
return castResult.error();
14861487
}
14871488

1488-
auto [ts, tzID] = castResult.value();
1489+
auto [ts, timeZone] = castResult.value();
14891490
// Input string may not contain a timezone - if so, it is interpreted in
14901491
// session timezone.
1491-
if (tzID == -1) {
1492-
tzID = sessionTzID_;
1492+
if (timeZone == nullptr) {
1493+
timeZone = sessionTimeZone_;
14931494
}
1494-
ts.toGMT(tzID);
1495-
result = pack(ts, tzID);
1495+
ts.toGMT(*timeZone);
1496+
result = pack(ts, timeZone->id());
14961497
return Status::OK();
14971498
}
14981499

14991500
private:
1500-
int16_t sessionTzID_{0};
1501+
const tz::TimeZone* sessionTimeZone_{tz::locateZone(0)}; // default to GMT.
15011502
};
15021503

15031504
template <typename T>
15041505
struct DateParseFunction {
15051506
VELOX_DEFINE_FUNCTION_TYPES(T);
15061507

15071508
std::shared_ptr<DateTimeFormatter> format_;
1508-
std::optional<int64_t> sessionTzID_;
1509+
1510+
// By default, assume 0 (GMT).
1511+
const tz::TimeZone* sessionTimeZone_{tz::locateZone(0)};
15091512
bool isConstFormat_ = false;
15101513

15111514
FOLLY_ALWAYS_INLINE void initialize(
@@ -1521,7 +1524,7 @@ struct DateParseFunction {
15211524

15221525
auto sessionTzName = config.sessionTimezone();
15231526
if (!sessionTzName.empty()) {
1524-
sessionTzID_ = tz::getTimeZoneID(sessionTzName);
1527+
sessionTimeZone_ = tz::locateZone(sessionTzName);
15251528
}
15261529
}
15271530

@@ -1539,10 +1542,7 @@ struct DateParseFunction {
15391542
return dateTimeResult.error();
15401543
}
15411544

1542-
// Since MySql format has no timezone specifier, simply check if session
1543-
// timezone was provided. If not, fallback to 0 (GMT).
1544-
int16_t timezoneId = sessionTzID_.value_or(0);
1545-
dateTimeResult->timestamp.toGMT(timezoneId);
1545+
dateTimeResult->timestamp.toGMT(*sessionTimeZone_);
15461546
result = dateTimeResult->timestamp;
15471547
return Status::OK();
15481548
}
@@ -1623,7 +1623,7 @@ struct ParseDateTimeFunction {
16231623
VELOX_DEFINE_FUNCTION_TYPES(T);
16241624

16251625
std::shared_ptr<DateTimeFormatter> format_;
1626-
std::optional<int64_t> sessionTzID_;
1626+
const tz::TimeZone* sessionTimeZone_{tz::locateZone(0)}; // GMT
16271627
bool isConstFormat_ = false;
16281628

16291629
FOLLY_ALWAYS_INLINE void initialize(
@@ -1639,7 +1639,7 @@ struct ParseDateTimeFunction {
16391639

16401640
auto sessionTzName = config.sessionTimezone();
16411641
if (!sessionTzName.empty()) {
1642-
sessionTzID_ = tz::getTimeZoneID(sessionTzName);
1642+
sessionTimeZone_ = tz::locateZone(sessionTzName);
16431643
}
16441644
}
16451645

@@ -1659,11 +1659,11 @@ struct ParseDateTimeFunction {
16591659

16601660
// If timezone was not parsed, fallback to the session timezone. If there's
16611661
// no session timezone, fallback to 0 (GMT).
1662-
int16_t timezoneId = dateTimeResult->timezoneId != -1
1663-
? dateTimeResult->timezoneId
1664-
: sessionTzID_.value_or(0);
1665-
dateTimeResult->timestamp.toGMT(timezoneId);
1666-
result = pack(dateTimeResult->timestamp, timezoneId);
1662+
const auto* timeZone = dateTimeResult->timezoneId != -1
1663+
? tz::locateZone(dateTimeResult->timezoneId)
1664+
: sessionTimeZone_;
1665+
dateTimeResult->timestamp.toGMT(*timeZone);
1666+
result = pack(dateTimeResult->timestamp, timeZone->id());
16671667
return Status::OK();
16681668
}
16691669
};

‎velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4461,14 +4461,14 @@ TEST_F(DateTimeFunctionsTest, toISO8601Timestamp) {
44614461

44624462
TEST_F(DateTimeFunctionsTest, toISO8601TimestampWithTimezone) {
44634463
const auto toIso = [&](const char* timestamp, const char* timezone) {
4464-
const auto tzId = tz::getTimeZoneID(timezone);
4464+
const auto* timeZone = tz::locateZone(timezone);
44654465
auto ts = parseTimestamp(timestamp);
4466-
ts.toGMT(tzId);
4466+
ts.toGMT(*timeZone);
44674467

44684468
return evaluateOnce<std::string>(
44694469
"to_iso8601(c0)",
44704470
TIMESTAMP_WITH_TIME_ZONE(),
4471-
std::make_optional(pack(ts.toMillis(), tzId)));
4471+
std::make_optional(pack(ts.toMillis(), timeZone->id())));
44724472
};
44734473

44744474
EXPECT_EQ(

‎velox/functions/prestosql/types/TimestampWithTimeZoneType.cpp

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,14 @@
2323
namespace facebook::velox {
2424
namespace {
2525

26-
int64_t getTimeZoneIDFromConfig(const core::QueryConfig& config) {
26+
const tz::TimeZone* getTimeZoneFromConfig(const core::QueryConfig& config) {
2727
const auto sessionTzName = config.sessionTimezone();
2828

2929
if (!sessionTzName.empty()) {
30-
return tz::getTimeZoneID(sessionTzName);
30+
return tz::locateZone(sessionTzName);
3131
}
3232

33-
return 0;
33+
return tz::locateZone(0); // GMT
3434
}
3535

3636
void castFromTimestamp(
@@ -39,7 +39,7 @@ void castFromTimestamp(
3939
const SelectivityVector& rows,
4040
int64_t* rawResults) {
4141
const auto& config = context.execCtx()->queryCtx()->queryConfig();
42-
int64_t sessionTzID = getTimeZoneIDFromConfig(config);
42+
const auto* sessionTimeZone = getTimeZoneFromConfig(config);
4343

4444
const auto adjustTimestampToTimezone = config.adjustTimestampToTimezone();
4545

@@ -49,9 +49,9 @@ void castFromTimestamp(
4949
// Treat TIMESTAMP as wall time in session time zone. This means that in
5050
// order to get its UTC representation we need to shift the value by the
5151
// offset of the time zone.
52-
ts.toGMT(sessionTzID);
52+
ts.toGMT(*sessionTimeZone);
5353
}
54-
rawResults[row] = pack(ts.toMillis(), sessionTzID);
54+
rawResults[row] = pack(ts.toMillis(), sessionTimeZone->id());
5555
});
5656
}
5757

@@ -61,17 +61,17 @@ void castFromDate(
6161
const SelectivityVector& rows,
6262
int64_t* rawResults) {
6363
const auto& config = context.execCtx()->queryCtx()->queryConfig();
64-
int64_t sessionTzID = getTimeZoneIDFromConfig(config);
64+
const auto* sessionTimeZone = getTimeZoneFromConfig(config);
6565

6666
static const int64_t kSecondsInDay = 86400;
6767

6868
context.applyToSelectedNoThrow(rows, [&](auto row) {
6969
const auto days = inputVector.valueAt(row);
7070
Timestamp ts{days * kSecondsInDay, 0};
71-
if (sessionTzID != 0) {
72-
ts.toGMT(sessionTzID);
71+
if (sessionTimeZone != nullptr) {
72+
ts.toGMT(*sessionTimeZone);
7373
}
74-
rawResults[row] = pack(ts.toMillis(), sessionTzID);
74+
rawResults[row] = pack(ts.toMillis(), sessionTimeZone->id());
7575
});
7676
}
7777

@@ -88,15 +88,15 @@ void castFromString(
8888
if (castResult.hasError()) {
8989
context.setStatus(row, castResult.error());
9090
} else {
91-
auto [ts, tzID] = castResult.value();
91+
auto [ts, timeZone] = castResult.value();
9292
// Input string may not contain a timezone - if so, it is interpreted in
9393
// session timezone.
94-
if (tzID == -1) {
94+
if (timeZone == nullptr) {
9595
const auto& config = context.execCtx()->queryCtx()->queryConfig();
96-
tzID = getTimeZoneIDFromConfig(config);
96+
timeZone = getTimeZoneFromConfig(config);
9797
}
98-
ts.toGMT(tzID);
99-
rawResults[row] = pack(ts.toMillis(), tzID);
98+
ts.toGMT(*timeZone);
99+
rawResults[row] = pack(ts.toMillis(), timeZone->id());
100100
}
101101
});
102102
}
@@ -146,7 +146,7 @@ void castToTimestamp(
146146
auto ts = unpackTimestampUtc(timestampWithTimezone);
147147
if (!adjustTimestampToTimezone) {
148148
// Convert UTC to the given time zone.
149-
ts.toTimezone(unpackZoneKeyId(timestampWithTimezone));
149+
ts.toTimezone(*tz::locateZone(unpackZoneKeyId(timestampWithTimezone)));
150150
}
151151
flatResult->set(row, ts);
152152
});
@@ -163,7 +163,8 @@ void castToDate(
163163
context.applyToSelectedNoThrow(rows, [&](auto row) {
164164
auto timestampWithTimezone = timestampVector->valueAt(row);
165165
auto timestamp = unpackTimestampUtc(timestampWithTimezone);
166-
timestamp.toTimezone(unpackZoneKeyId(timestampWithTimezone));
166+
timestamp.toTimezone(
167+
*tz::locateZone(unpackZoneKeyId(timestampWithTimezone)));
167168

168169
const auto days = util::toDate(timestamp, nullptr);
169170
flatResult->set(row, days);

0 commit comments

Comments
 (0)