facebookincubator / velox

A composable and fully extensible C++ execution engine library for data management systems.
https://velox-lib.io/
Apache License 2.0
3.51k stars 1.15k forks source link

Support parsing session time zone like "-08:00" #7804

Open PHILO-HE opened 11 months ago

PHILO-HE commented 11 months ago

Description

For functions using getTimeZoneFromConfig, we found velox cannot recognize some session time zone configured like "-08:00".

mbasmanova commented 11 months ago

CC: @pedroerp

pedroerp commented 11 months ago

That might be true. getTimeZoneFromConfig() relies on the external date library (which looks it up based on your OS timezone database), and it might be that it does not support that pattern. Do you have a way to reproduce the error?

PHILO-HE commented 11 months ago

Hi @pedroerp, this is a small test with such timezone pattern used.

diff --git a/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp b/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp
index af05d1c9c..54661cb2a 100644
--- a/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp
+++ b/velox/functions/prestosql/tests/DateTimeFunctionsTest.cpp
@@ -2596,6 +2596,12 @@ TEST_F(DateTimeFunctionsTest, parseDatetime) {
       parseDatetime("1969-12-31+07:30+02:00", "YYYY-MM-dd+HH:mmZZ"));
 }

+TEST_F(DateTimeFunctionsTest, useUnsupportedTZ) {
+  using util::fromTimestampString;
+  setQueryTimeZone("-08:00");
+  formatDatetime(fromTimestampString("1970-01-01"), "yyyy");
+}
+
 TEST_F(DateTimeFunctionsTest, formatDateTime) {
   using util::fromTimestampString;

Here is the error log:

[ RUN      ] DateTimeFunctionsTest.useUnsupportedTZ
unknown file: Failure
C++ exception with description "Exception: VeloxUserError
Error Source: USER
Error Code: INVALID_ARGUMENT
Reason: -08:00 not found in timezone database
Retriable: False
Context: format_datetime(c0, c1)
Top-Level Context: Same as context.
Function: 
File: UNKNOWN
Stack trace:
Stack trace has been disabled. Use --velox_exception_user_stacktrace_enabled=true to enable it.
" thrown in the test body.
[  FAILED  ] DateTimeFunctionsTest.useUnsupportedTZ (4 ms)