Skip to content

Commit

Permalink
Add allowOverflow flag to Timestamp::toTimezone (#9836)
Browse files Browse the repository at this point in the history
Summary:
Allow integer overflow when converting timestamp to TimePoint.

Use in Spark function from_unixtime that allows overflow.

Fixes #9778

Pull Request resolved: #9836

Reviewed By: amitkdutta

Differential Revision: D57659010

Pulled By: mbasmanova

fbshipit-source-id: d60fd686c19dff02eebba1a0c0410056d189c6f5
  • Loading branch information
NEUpanning authored and facebook-github-bot committed May 22, 2024
1 parent 675b6dd commit 47fc3dd
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 10 deletions.
6 changes: 6 additions & 0 deletions velox/docs/functions/spark/datetime.rst
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,12 @@ These functions support TIMESTAMP and DATE input types.
SELECT from_unixtime(3600, 'yyyy'); -- '1970'
SELECT from_unixtime(9223372036854775807, "yyyy-MM-dd HH:mm:ss"); -- '1969-12-31 23:59:59'

If we run the following query in the `Asia/Shanghai` time zone: ::

SELECT from_unixtime(100, 'yyyy-MM-dd HH:mm:ss'); -- '1970-01-01 08:01:40'
SELECT from_unixtime(3600, 'yyyy'); -- '1970'
SELECT from_unixtime(9223372036854775807, "yyyy-MM-dd HH:mm:ss"); -- '1970-01-01 07:59:59'

.. spark:function:: from_utc_timestamp(timestamp, string) -> timestamp
Returns the timestamp value from UTC timezone to the given timezone. ::
Expand Down
2 changes: 1 addition & 1 deletion velox/functions/lib/DateTimeFormatter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1033,7 +1033,7 @@ int32_t DateTimeFormatter::format(
bool allowOverflow) const {
Timestamp t = timestamp;
if (timezone != nullptr) {
t.toTimezone(*timezone);
t.toTimezone(*timezone, allowOverflow);
}
const auto timePoint = t.toTimePoint(allowOverflow);
const auto daysTimePoint = date::floor<date::days>(timePoint);
Expand Down
9 changes: 9 additions & 0 deletions velox/functions/sparksql/tests/DateTimeFunctionsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -898,6 +898,15 @@ TEST_F(DateTimeFunctionsTest, fromUnixtime) {

// 8 hours ahead UTC.
setQueryTimeZone("Asia/Shanghai");
// In debug mode, Timestamp constructor will throw exception if range check
// fails.
#ifdef NDEBUG
// Integer overflow in the internal conversion from seconds to milliseconds.
EXPECT_EQ(
fromUnixTime(std::numeric_limits<int64_t>::max(), "yyyy-MM-dd HH:mm:ss"),
"1970-01-01 07:59:59");
#endif

EXPECT_EQ(fromUnixTime(0, "yyyy-MM-dd HH:mm:ss"), "1970-01-01 08:00:00");
EXPECT_EQ(fromUnixTime(120, "yyyy-MM-dd HH:mm"), "1970-01-01 08:02");
EXPECT_EQ(fromUnixTime(-59, "yyyy-MM-dd HH:mm:ss"), "1970-01-01 07:59:01");
Expand Down
4 changes: 2 additions & 2 deletions velox/type/Timestamp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ Timestamp::toTimePoint(bool allowOverflow) const {
return tp;
}

void Timestamp::toTimezone(const date::time_zone& zone) {
auto tp = toTimePoint();
void Timestamp::toTimezone(const date::time_zone& zone, bool allowOverflow) {
auto tp = toTimePoint(allowOverflow);
auto epoch = zone.to_local(tp).time_since_epoch();
// NOTE: Round down to get the seconds of the current time point.
seconds_ = std::chrono::floor<std::chrono::seconds>(epoch).count();
Expand Down
16 changes: 9 additions & 7 deletions velox/type/Timestamp.h
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ struct Timestamp {
/// Due to the limit of std::chrono, throws if timestamp is outside of
/// [-32767-01-01, 32767-12-31] range.
/// If allowOverflow is true, integer overflow is allowed in converting
/// timestmap to milliseconds.
/// timestamp to milliseconds.
std::chrono::time_point<std::chrono::system_clock, std::chrono::milliseconds>
toTimePoint(bool allowOverflow = false) const;

Expand Down Expand Up @@ -311,12 +311,14 @@ struct Timestamp {
// Same as above, but accepts PrestoDB time zone ID.
void toGMT(int16_t tzID);

// Assuming the timestamp represents a GMT time, converts it to the time at
// the same moment at zone.
// Example: Timestamp ts{0, 0};
// ts.Timezone("America/Los_Angeles");
// ts.toString() returns December 31, 1969 16:00:00
void toTimezone(const date::time_zone& zone);
/// Assuming the timestamp represents a GMT time, converts it to the time at
/// the same moment at zone.
/// @param allowOverflow If true, integer overflow is allowed when converting
/// timestamp to TimePoint. Otherwise, user exception is thrown for overflow.
/// Example: Timestamp ts{0, 0};
/// ts.Timezone("America/Los_Angeles");
/// ts.toString() returns December 31, 1969 16:00:00
void toTimezone(const date::time_zone& zone, bool allowOverflow = false);

// Same as above, but accepts PrestoDB time zone ID.
void toTimezone(int16_t tzID);
Expand Down

0 comments on commit 47fc3dd

Please sign in to comment.