Skip to content

Commit

Permalink
Remove old time zone ID based API from Timestamp
Browse files Browse the repository at this point in the history
Summary:
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.

Differential Revision: D60477527
  • Loading branch information
pedroerp authored and facebook-github-bot committed Aug 1, 2024
1 parent 97ad9ad commit f59b3af
Show file tree
Hide file tree
Showing 13 changed files with 165 additions and 198 deletions.
4 changes: 2 additions & 2 deletions velox/expression/PrestoCastHooks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ Expected<Timestamp> PrestoCastHooks::castStringToTimestamp(
// If the parsed string has timezone information, convert the timestamp at
// GMT at that time. For example, "1970-01-01 00:00:00 -00:01" is 60 seconds
// at GMT.
if (result.second != -1) {
result.first.toGMT(result.second);
if (result.second != nullptr) {
result.first.toGMT(*result.second);

}
// If no timezone information is available in the input string, check if we
Expand Down
46 changes: 23 additions & 23 deletions velox/functions/prestosql/DateTimeFunctions.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ struct TimestampWithTimezoneSupport {
bool asGMT = false) {
auto timestamp = unpackTimestampUtc(timestampWithTimezone);
if (!asGMT) {
timestamp.toTimezone(unpackZoneKeyId(timestampWithTimezone));
timestamp.toTimezone(
*tz::locateZone(unpackZoneKeyId(timestampWithTimezone)));
}

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

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

result = pack(timestamp, unpackZoneKeyId(timestampWithTimezone));
}
Expand Down Expand Up @@ -1472,7 +1473,7 @@ struct FromIso8601Timestamp {
const arg_type<Varchar>* /*input*/) {
auto sessionTzName = config.sessionTimezone();
if (!sessionTzName.empty()) {
sessionTzID_ = tz::getTimeZoneID(sessionTzName);
sessionTimeZone_ = tz::locateZone(sessionTzName);
}
}

Expand All @@ -1485,27 +1486,29 @@ struct FromIso8601Timestamp {
return castResult.error();
}

auto [ts, tzID] = castResult.value();
auto [ts, timeZone] = castResult.value();
// Input string may not contain a timezone - if so, it is interpreted in
// session timezone.
if (tzID == -1) {
tzID = sessionTzID_;
if (timeZone == nullptr) {
timeZone = sessionTimeZone_;
}
ts.toGMT(tzID);
result = pack(ts, tzID);
ts.toGMT(*timeZone);
result = pack(ts, timeZone->id());
return Status::OK();
}

private:
int16_t sessionTzID_{0};
const tz::TimeZone* sessionTimeZone_{tz::locateZone(0)}; // default to GMT.
};

template <typename T>
struct DateParseFunction {
VELOX_DEFINE_FUNCTION_TYPES(T);

std::shared_ptr<DateTimeFormatter> format_;
std::optional<int64_t> sessionTzID_;

// By default, assume 0 (GMT).
const tz::TimeZone* sessionTimeZone_{tz::locateZone(0)};
bool isConstFormat_ = false;

FOLLY_ALWAYS_INLINE void initialize(
Expand All @@ -1521,7 +1524,7 @@ struct DateParseFunction {

auto sessionTzName = config.sessionTimezone();
if (!sessionTzName.empty()) {
sessionTzID_ = tz::getTimeZoneID(sessionTzName);
sessionTimeZone_ = tz::locateZone(sessionTzName);
}
}

Expand All @@ -1539,10 +1542,7 @@ struct DateParseFunction {
return dateTimeResult.error();
}

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

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

FOLLY_ALWAYS_INLINE void initialize(
Expand All @@ -1639,7 +1639,7 @@ struct ParseDateTimeFunction {

auto sessionTzName = config.sessionTimezone();
if (!sessionTzName.empty()) {
sessionTzID_ = tz::getTimeZoneID(sessionTzName);
sessionTimeZone_ = tz::locateZone(sessionTzName);
}
}

Expand All @@ -1659,11 +1659,11 @@ struct ParseDateTimeFunction {

// If timezone was not parsed, fallback to the session timezone. If there's
// no session timezone, fallback to 0 (GMT).
int16_t timezoneId = dateTimeResult->timezoneId != -1
? dateTimeResult->timezoneId
: sessionTzID_.value_or(0);
dateTimeResult->timestamp.toGMT(timezoneId);
result = pack(dateTimeResult->timestamp, timezoneId);
const auto* timeZone = dateTimeResult->timezoneId != -1
? tz::locateZone(dateTimeResult->timezoneId)
: sessionTimeZone_;
dateTimeResult->timestamp.toGMT(*timeZone);
result = pack(dateTimeResult->timestamp, timeZone->id());
return Status::OK();
}
};
Expand Down
6 changes: 3 additions & 3 deletions velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4461,14 +4461,14 @@ TEST_F(DateTimeFunctionsTest, toISO8601Timestamp) {

TEST_F(DateTimeFunctionsTest, toISO8601TimestampWithTimezone) {
const auto toIso = [&](const char* timestamp, const char* timezone) {
const auto tzId = tz::getTimeZoneID(timezone);
const auto* timeZone = tz::locateZone(timezone);
auto ts = parseTimestamp(timestamp);
ts.toGMT(tzId);
ts.toGMT(*timeZone);

return evaluateOnce<std::string>(
"to_iso8601(c0)",
TIMESTAMP_WITH_TIME_ZONE(),
std::make_optional(pack(ts.toMillis(), tzId)));
std::make_optional(pack(ts.toMillis(), timeZone->id())));
};

EXPECT_EQ(
Expand Down
35 changes: 18 additions & 17 deletions velox/functions/prestosql/types/TimestampWithTimeZoneType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@
namespace facebook::velox {
namespace {

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

if (!sessionTzName.empty()) {
return tz::getTimeZoneID(sessionTzName);
return tz::locateZone(sessionTzName);
}

return 0;
return tz::locateZone(0); // GMT
}

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

const auto adjustTimestampToTimezone = config.adjustTimestampToTimezone();

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

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

static const int64_t kSecondsInDay = 86400;

context.applyToSelectedNoThrow(rows, [&](auto row) {
const auto days = inputVector.valueAt(row);
Timestamp ts{days * kSecondsInDay, 0};
if (sessionTzID != 0) {
ts.toGMT(sessionTzID);
if (sessionTimeZone != nullptr) {
ts.toGMT(*sessionTimeZone);
}
rawResults[row] = pack(ts.toMillis(), sessionTzID);
rawResults[row] = pack(ts.toMillis(), sessionTimeZone->id());
});
}

Expand All @@ -88,15 +88,15 @@ void castFromString(
if (castResult.hasError()) {
context.setStatus(row, castResult.error());
} else {
auto [ts, tzID] = castResult.value();
auto [ts, timeZone] = castResult.value();
// Input string may not contain a timezone - if so, it is interpreted in
// session timezone.
if (tzID == -1) {
if (timeZone == nullptr) {
const auto& config = context.execCtx()->queryCtx()->queryConfig();
tzID = getTimeZoneIDFromConfig(config);
timeZone = getTimeZoneFromConfig(config);
}
ts.toGMT(tzID);
rawResults[row] = pack(ts.toMillis(), tzID);
ts.toGMT(*timeZone);
rawResults[row] = pack(ts.toMillis(), timeZone->id());
}
});
}
Expand Down Expand Up @@ -146,7 +146,7 @@ void castToTimestamp(
auto ts = unpackTimestampUtc(timestampWithTimezone);
if (!adjustTimestampToTimezone) {
// Convert UTC to the given time zone.
ts.toTimezone(unpackZoneKeyId(timestampWithTimezone));
ts.toTimezone(*tz::locateZone(unpackZoneKeyId(timestampWithTimezone)));
}
flatResult->set(row, ts);
});
Expand All @@ -163,7 +163,8 @@ void castToDate(
context.applyToSelectedNoThrow(rows, [&](auto row) {
auto timestampWithTimezone = timestampVector->valueAt(row);
auto timestamp = unpackTimestampUtc(timestampWithTimezone);
timestamp.toTimezone(unpackZoneKeyId(timestampWithTimezone));
timestamp.toTimezone(
*tz::locateZone(unpackZoneKeyId(timestampWithTimezone)));

const auto days = util::toDate(timestamp, nullptr);
flatResult->set(row, days);
Expand Down
Loading

0 comments on commit f59b3af

Please sign in to comment.