Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@marin-ma
marin-ma committed Feb 23, 2024
1 parent d2bfec5 commit 6baeb25
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 68 deletions.
36 changes: 18 additions & 18 deletions velox/docs/functions/spark/datetime.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,24 @@ These functions support TIMESTAMP and DATE input types.

SELECT quarter('2009-07-30'); -- 3

.. spark:function:: make_timestamp(year, month, day, hour, min, sec[, timezone]) -> timestamp
Create timestamp from ``year``, ``month``, ``day``, ``hour``, ``min`` and ``sec`` fields.
If the ``timezone`` parameter is provided,
the function interprets the input time components as being in the specified ``timezone``.
Otherwise if ``session_timezone`` is configured,
the function assumes the inputs are in the session's configured time zone.
If neither the ``timezone`` parameter nor the ``session_timezone`` configuration is specified,
an exception will be thrown.

Returns the timestamp adjusted to the GMT time zone. ::

SELECT make_timestamp(2014, 12, 28, 6, 30, 45.887); -- 2014-12-28 06:30:45.887
SELECT make_timestamp(2014, 12, 28, 6, 30, 45.887, 'CET'); -- 2014-12-28 05:30:45.887
SELECT make_timestamp(2019, 6, 30, 23, 59, 60); -- 2019-07-01 00:00:00
SELECT make_timestamp(2019, 6, 30, 23, 59, 1); -- 2019-06-30 23:59:01
SELECT make_timestamp(null, 7, 22, 15, 30, 0); -- NULL

.. spark:function:: month(date) -> integer
Returns the month of ``date``. ::
Expand Down Expand Up @@ -201,21 +219,3 @@ These functions support TIMESTAMP and DATE input types.
.. spark:function:: year(x) -> integer
Returns the year from ``x``.

.. spark:function:: make_timestamp(year, month, day, hour, min, sec[, timezone]) -> timestamp
Create timestamp from ``year``, ``month``, ``day``, ``hour``, ``min`` and ``sec`` fields.
If the ``timezone`` parameter is provided,
the function interprets the input time components as being in the specified ``timezone``.
Otherwise if ``session_timezone`` is configured,
the function assumes the inputs are in the session's configured time zone.
If neither the ``timezone`` parameter nor the ``session_timezone`` configuration is specified,
the inputs are treated as in the GMT timezone.

Returns the timestamp adjusted to the GMT time zone. ::

SELECT make_timestamp(2014, 12, 28, 6, 30, 45.887); -- 2014-12-28 06:30:45.887
SELECT make_timestamp(2014, 12, 28, 6, 30, 45.887, 'CET'); -- 2014-12-28 05:30:45.887
SELECT make_timestamp(2019, 6, 30, 23, 59, 60); -- 2019-07-01 00:00:00
SELECT make_timestamp(2019, 6, 30, 23, 59, 1); -- 2019-06-30 23:59:01
SELECT make_timestamp(null, 7, 22, 15, 30, 0); -- NULL
20 changes: 10 additions & 10 deletions velox/functions/sparksql/DateTimeFunctions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,11 @@ std::optional<Timestamp> makeTimeStampFromDecodedArgs(
yearVector->valueAt<int32_t>(row),
monthVector->valueAt<int32_t>(row),
dayVector->valueAt<int32_t>(row));
auto localMicros =
hour * util::kMicrosPerHour + minute * util::kMicrosPerMinute + micros;
// micros has at most 8 digits (2 for seconds + 6 for microseconds),
// thus it's safe to cast micros from int64_t to int32_t.
auto localMicros = util::fromTime(hour, minute, 0, (int32_t)micros);
return util::fromDatetime(daysSinceEpoch, localMicros);
} catch (const VeloxException& e) {
if (!e.isUserError()) {
throw;
}
} catch (const VeloxUserError& e) {
return std::nullopt;
} catch (const std::exception&) {
throw;
Expand Down Expand Up @@ -173,6 +171,12 @@ std::shared_ptr<exec::VectorFunction> createMakeTimestampFunction(
const std::string& /* name */,
const std::vector<exec::VectorFunctionArg>& inputArgs,
const core::QueryConfig& config) {
const auto sessionTzName = config.sessionTimezone();
VELOX_USER_CHECK(
!sessionTzName.empty(),
"make_timestamp requires session time zone to be set.")
const auto sessionTzID = util::getTimeZoneID(sessionTzName);

VELOX_USER_CHECK(
inputArgs[5].type->isShortDecimal(),
"Seconds must be short decimal type but got {}",
Expand All @@ -183,10 +187,6 @@ std::shared_ptr<exec::VectorFunction> createMakeTimestampFunction(
"Seconds fraction must have 6 digits for microseconds but got {}",
microsType.scale());

const auto sessionTzName = config.sessionTimezone();
const auto sessionTzID =
sessionTzName.empty() ? 0 : util::getTimeZoneID(sessionTzName);

return std::make_shared<MakeTimestampFunction>(sessionTzID);
}
} // namespace
Expand Down
72 changes: 32 additions & 40 deletions velox/functions/sparksql/tests/DateTimeFunctionsTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -790,8 +790,7 @@ TEST_F(DateTimeFunctionsTest, makeTimestamp) {
{45678000, 1e6, 6e7, 59999999, std::nullopt}, microsType);
auto data = makeRowVector({year, month, day, hour, minute, micros});

// Test w/o session time zone.
setQueryTimeZone("");
setQueryTimeZone("GMT");
auto expectedGMT = makeNullableFlatVector<Timestamp>(
{util::fromTimestampString("2021-07-11 06:30:45.678"),
util::fromTimestampString("2021-07-11 06:30:01"),
Expand All @@ -801,7 +800,6 @@ TEST_F(DateTimeFunctionsTest, makeTimestamp) {
testMakeTimestamp(data, expectedGMT, false);
testConstantTimezone(data, "GMT", expectedGMT);

// Test w/ session time zone.
setQueryTimeZone("Asia/Shanghai");
auto expectedSessionTimezone = makeNullableFlatVector<Timestamp>(
{util::fromTimestampString("2021-07-10 22:30:45.678"),
Expand All @@ -810,11 +808,13 @@ TEST_F(DateTimeFunctionsTest, makeTimestamp) {
util::fromTimestampString("2021-07-10 22:30:59.999999"),
std::nullopt});
testMakeTimestamp(data, expectedSessionTimezone, false);
// Session time zone will be ignored if time zone is specified in argument.
testConstantTimezone(data, "GMT", expectedGMT);
}

// Valid cases w/ time zone argument.
{
setQueryTimeZone("Asia/Shanghai");
const auto year = makeFlatVector<int32_t>({2021, 2021, 1});
const auto month = makeFlatVector<int32_t>({07, 07, 1});
const auto day = makeFlatVector<int32_t>({11, 11, 1});
Expand All @@ -826,28 +826,39 @@ TEST_F(DateTimeFunctionsTest, makeTimestamp) {
makeNullableFlatVector<StringView>({"GMT", "CET", std::nullopt});
auto data =
makeRowVector({year, month, day, hour, minute, micros, timeZone});
{
setQueryTimeZone("");
auto expected = makeNullableFlatVector<Timestamp>(
{util::fromTimestampString("2021-07-11 06:30:45.678"),
util::fromTimestampString("2021-07-11 04:30:45.678"),
std::nullopt});
testMakeTimestamp(data, expected, true);
}
{
// Session time zone will be ignored if time zone is specified in
// argument.
setQueryTimeZone("Asia/Shanghai");
auto expected = makeNullableFlatVector<Timestamp>(
{util::fromTimestampString("2021-07-11 06:30:45.678"),
util::fromTimestampString("2021-07-11 04:30:45.678"),
std::nullopt});
testMakeTimestamp(data, expected, true);
}
// Session time zone will be ignored if time zone is specified in argument.
auto expected = makeNullableFlatVector<Timestamp>(
{util::fromTimestampString("2021-07-11 06:30:45.678"),
util::fromTimestampString("2021-07-11 04:30:45.678"),
std::nullopt});
testMakeTimestamp(data, expected, true);
}

// Invalid cases.
{
const auto testInvalid = [&](int64_t microsec, const TypePtr& microsType) {
return evaluateOnce<Timestamp, int64_t>(
"make_timestamp(c0, c1, c2, c3, c4, c5)",
{1, 1, 1, 1, 1, microsec},
{INTEGER(), INTEGER(), INTEGER(), INTEGER(), INTEGER(), microsType});
};

// No session time zone.
setQueryTimeZone("");
VELOX_ASSERT_THROW(
testInvalid(60007000, DECIMAL(16, 6)),
"make_timestamp requires session time zone to be set.");

// Invalid data type for microseconds.
setQueryTimeZone("Asia/Shanghai");
VELOX_ASSERT_THROW(
testInvalid(1e6, DECIMAL(20, 8)),
"Seconds must be short decimal type but got DECIMAL(20, 8)");
VELOX_ASSERT_THROW(
testInvalid(60007000, DECIMAL(16, 8)),
"Seconds fraction must have 6 digits for microseconds but got 8");

// Invalid arguments returns null.
const auto year = makeFlatVector<int32_t>(
{facebook::velox::util::kMinYear - 1,
facebook::velox::util::kMaxYear + 1,
Expand Down Expand Up @@ -886,25 +897,6 @@ TEST_F(DateTimeFunctionsTest, makeTimestamp) {
testInvalidMicros(99999999);
testInvalidMicros(999999999);
testInvalidMicros(60007000);

const auto testMicrosError = [&](int64_t microsec,
const TypePtr& microsType,
const std::string& expectedError) {
const auto micros =
makeFlatVector<int64_t>(std::vector<int64_t>{microsec}, microsType);
auto data = makeRowVector({year, month, day, hour, minute, micros});
VELOX_ASSERT_THROW(
evaluate("make_timestamp(c0, c1, c2, c3, c4, c5)", data),
expectedError);
};
testMicrosError(
60007000,
DECIMAL(20, 8),
"Seconds must be short decimal type but got DECIMAL(20, 8)");
testMicrosError(
60007000,
DECIMAL(18, 8),
"Seconds fraction must have 6 digits for microseconds but got 8");
}
}

Expand Down

0 comments on commit 6baeb25

Please sign in to comment.