You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/06/23 14:35:53 UTC

[GitHub] [arrow] palak-9202 opened a new pull request, #13428: adding utc <-> timezone conversion functions

palak-9202 opened a new pull request, #13428:
URL: https://github.com/apache/arrow/pull/13428

   Adding functions 
   to_utc_timezone : Converts a timestamp from local timezone to UTC time
   from_utc_timezone : Converts a timestamp from UTC time to local time


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] anthonylouisbsb commented on a diff in pull request #13428: ARROW-16918: [Gandiva] [C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
anthonylouisbsb commented on code in PR #13428:
URL: https://github.com/apache/arrow/pull/13428#discussion_r914119747


##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,47 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using arrow_vendored::date::time_zone;
+
+  arrow_vendored::date::sys_time <std::chrono::milliseconds> tp
+                                  {std::chrono::milliseconds{time_miliseconds}};
+  try {
+    const time_zone* local_tz = arrow_vendored::date::locate_zone
+                                (std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count()*1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+      gdv_fn_context_set_error_msg(context, "Invalid time zone");

Review Comment:
   As it is an implementation of a Hive function, do you know if there is a way to print all available timezones for the arrow_vendored library? Is just to check the compatibility of that function with all available timezones of Hive: http://obscuredclarity.blogspot.com/2010/08/get-all-timezones-available-in-timezone.html



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] projjal commented on a diff in pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
projjal commented on code in PR #13428:
URL: https://github.com/apache/arrow/pull/13428#discussion_r924337280


##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,58 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using arrow_vendored::date::locate_zone;
+  using arrow_vendored::date::sys_time;
+  using std::chrono::milliseconds;
+
+  sys_time<milliseconds> tp{milliseconds{time_miliseconds}};
+  try {
+    const auto local_tz = locate_zone(std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count() * 1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch (...) {
+    auto msg_len = static_cast<int32_t>(strlen(timezone) + 50);

Review Comment:
   you can't use strlen since timezone is not a null terminated string. You can simply do the following
   ```
   std::string err_msg = std::string(timezone, length) + " is an invalid time zone name.";
   gdv_fn_context_set_error_msg(context, err_msg.c_str());
   return 0;



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] palak-9202 commented on a diff in pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
palak-9202 commented on code in PR #13428:
URL: https://github.com/apache/arrow/pull/13428#discussion_r917534784


##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,51 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::locate_zone;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  try {
+    const time_zone* local_tz = locate_zone(std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count()*1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+      gdv_fn_context_set_error_msg(context, "Invalid time zone");
+      return 0;
+  }
+}
+
+GANDIVA_EXPORT
+gdv_timestamp from_utc_timezone_timestamp(gdv_int64 context,
+                                          gdv_timestamp time_miliseconds,
+                                          const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::make_zoned;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  const zoned_time<milliseconds, const time_zone*> utc_tz =
+                      make_zoned(std::string("Etc/UTC"), tp);
+  try {
+    const zoned_time<milliseconds, const time_zone*> local_tz =
+                      make_zoned(std::string(timezone, length), utc_tz);
+    gdv_timestamp offset = local_tz.get_time_zone()->get_info(tp).offset.count()*1000;
+    return time_miliseconds + static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+      gdv_fn_context_set_error_msg(context, "Invalid time zone");

Review Comment:
   Sure



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] palak-9202 commented on a diff in pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
palak-9202 commented on code in PR #13428:
URL: https://github.com/apache/arrow/pull/13428#discussion_r920799751


##########
cpp/src/gandiva/gdv_function_stubs_test.cc:
##########
@@ -993,4 +993,61 @@ TEST(TestGdvFnStubs, TestTranslate) {
   EXPECT_EQ(expected, std::string(result, out_len));
 }
 
+TEST(TestGdvFnStubs, TestToUtcTimezone) {
+  gandiva::ExecutionContext context;
+  auto context_ptr = reinterpret_cast<int64_t>(&context);
+  gdv_int32 len_ist = static_cast<gdv_int32>(strlen("Asia/Kolkata"));
+  gdv_int32 len_pst = static_cast<gdv_int32>(strlen("America/Los_Angeles"));
+
+  //2012-02-28 15:30:00
+  gdv_timestamp ts = 1330443000000;
+  gdv_timestamp ts2 =
+      to_utc_timezone_timestamp(context_ptr, ts, "Asia/Kolkata", len_ist);
+  EXPECT_EQ(1330423200000, ts2);
+
+  //1970-01-01 5:00:00
+  ts = 18000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "Asia/Kolkata", len_ist);
+  EXPECT_EQ(ts2, -1800000);
+
+  //daylight savings check
+  //2018-03-11 01:00:00
+  ts = 	1520730000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "America/Los_Angeles", len_pst);
+  EXPECT_EQ(ts2, 1520758800000);
+
+  //2018-03-12 01:00:00
+  ts = 1331712000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "America/Los_Angeles", len_pst);
+  EXPECT_EQ(ts2, 1331737200000);
+
+  //Failure case
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "America/LA", 10);
+  EXPECT_THAT(context.get_error(), "'America/LA' is an invalid time zone name.");
+}
+
+TEST(TestGdvFnStubs, TestFromUtcTimezone) {
+  ExecutionContext context;
+  auto context_ptr = reinterpret_cast<int64_t>(&context);
+  gdv_int32 len_ist = static_cast<gdv_int32>(strlen("Asia/Kolkata"));
+  gdv_int32 len_pst = static_cast<gdv_int32>(strlen("America/Los_Angeles"));
+
+  gdv_timestamp ts = 36000000;
+  gdv_timestamp ts2 =
+      from_utc_timezone_timestamp(context_ptr, ts, "Asia/Kolkata", len_ist);
+  EXPECT_EQ(ts2, 55800000);
+
+  ts = -1800000;
+  ts2 = from_utc_timezone_timestamp(context_ptr, ts, "Asia/Kolkata", len_ist);
+  EXPECT_EQ(ts2, 18000000);
+
+  ts = 1520758800000;
+  ts2 = from_utc_timezone_timestamp(context_ptr, ts, "America/Los_Angeles", len_pst);
+  EXPECT_EQ(ts2, 1520730000000);
+
+  ts = 1331737200000;
+  ts2 = from_utc_timezone_timestamp(context_ptr, ts, "America/Los_Angeles", len_pst);
+  EXPECT_EQ(ts2, 1331712000000);
+}

Review Comment:
   I've added all the suggested changes and corrections



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] palak-9202 commented on a diff in pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
palak-9202 commented on code in PR #13428:
URL: https://github.com/apache/arrow/pull/13428#discussion_r919738853


##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,51 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::locate_zone;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  try {
+    const time_zone* local_tz = locate_zone(std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count()*1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+      gdv_fn_context_set_error_msg(context, "Invalid time zone");
+      return 0;
+  }
+}
+
+GANDIVA_EXPORT
+gdv_timestamp from_utc_timezone_timestamp(gdv_int64 context,
+                                          gdv_timestamp time_miliseconds,
+                                          const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::make_zoned;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  const zoned_time<milliseconds, const time_zone*> utc_tz =

Review Comment:
   Yes



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] ursabot commented on pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #13428:
URL: https://github.com/apache/arrow/pull/13428#issuecomment-1190993178

   Benchmark runs are scheduled for baseline = 0ebaad9105e976adcb80582255c4f07fb12548dd and contender = 85c0db74130394614dfdc41bece179bfd349d0dd. 85c0db74130394614dfdc41bece179bfd349d0dd is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Failed :arrow_down:1.79% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/53a4fcb244f44059868fee1931e6d364...a180babf76f24b30bec11a9714aad038/)
   [Failed :arrow_down:0.38% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/8140a74cf694469b84f6b8dbc83c9e5d...a47acdf898c241e695022d2da5a0cb12/)
   [Failed] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/8c886af9e02e4b5582aa513106b87311...40c642e6ff8144c090a8eb9e533c57e2/)
   [Finished :arrow_down:0.36% :arrow_up:0.04%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/6f8aad2dc9234c99a6f3501f909fa643...10ee5bb7ab75484686424fba3040deff/)
   Buildkite builds:
   [Failed] [`85c0db74` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1153)
   [Failed] [`85c0db74` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1158)
   [Failed] [`85c0db74` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1145)
   [Finished] [`85c0db74` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1164)
   [Failed] [`0ebaad91` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1152)
   [Failed] [`0ebaad91` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1157)
   [Failed] [`0ebaad91` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1144)
   [Finished] [`0ebaad91` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1161)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou commented on a diff in pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
kou commented on code in PR #13428:
URL: https://github.com/apache/arrow/pull/13428#discussion_r919776430


##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,51 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::locate_zone;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  try {
+    const time_zone* local_tz = locate_zone(std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count()*1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+      gdv_fn_context_set_error_msg(context, "Invalid time zone");
+      return 0;
+  }
+}
+
+GANDIVA_EXPORT
+gdv_timestamp from_utc_timezone_timestamp(gdv_int64 context,
+                                          gdv_timestamp time_miliseconds,
+                                          const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::make_zoned;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  const zoned_time<milliseconds, const time_zone*> utc_tz =

Review Comment:
   It seems that https://github.com/palak-9202/arrow/commit/4c0549526e56866205e4b1baa993b2e3e67241d5#diff-6112a00ef24accb25d6dde3f15276ee354c695838994d2e80e0de65c0e1929abL100 is the problem. https://github.com/palak-9202/arrow/commit/4c0549526e56866205e4b1baa993b2e3e67241d5#diff-6112a00ef24accb25d6dde3f15276ee354c695838994d2e80e0de65c0e1929abL102 didn't cause the problem.
   The former uses `const char *` and the latter uses `std::string`.
   How about trying the above diff that uses `std::string` for the former?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou commented on a diff in pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
kou commented on code in PR #13428:
URL: https://github.com/apache/arrow/pull/13428#discussion_r917208665


##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,51 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::locate_zone;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  try {
+    const time_zone* local_tz = locate_zone(std::string(timezone, length));

Review Comment:
   ```suggestion
       const auto local_tz = locate_zone(std::string(timezone, length));
   ```



##########
cpp/src/gandiva/gdv_function_stubs_test.cc:
##########
@@ -993,4 +993,57 @@ TEST(TestGdvFnStubs, TestTranslate) {
   EXPECT_EQ(expected, std::string(result, out_len));
 }
 
+TEST(TestTime, TestToUtcTimezone) {
+  ExecutionContext context;
+  auto context_ptr = reinterpret_cast<int64_t>(&context);
+  gdv_int32 len_ist = static_cast<gdv_int32>(strlen("Asia/Kolkata"));
+  gdv_int32 len_pst = static_cast<gdv_int32>(strlen("America/Los_Angeles"));
+
+  //2012-02-28 15:30:00 Asia/Kolkata
+  gdv_timestamp ts = 55800000;
+  gdv_timestamp ts2 = to_utc_timezone_timestamp(context_ptr, ts,
+                                                "Asia/Kolkata", len_ist);
+  EXPECT_EQ(36000000, ts2);
+
+  //1970-01-01 5:00:00 Asia/Kolkata
+  ts = 18000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "Asia/Kolkata", len_ist);
+  EXPECT_EQ(ts2, -1800000);
+
+  //daylight savings check
+  //2018-03-11 01:00:00 America/Los_Angeles
+  ts = 	1520730000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "America/Los_Angeles", len_pst);
+  EXPECT_EQ(ts2, 1520758800000);
+
+  //2018-03-12 01:00:00 America/Los_Angeles
+  ts = 1331712000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "America/Los_Angeles", len_pst);
+  EXPECT_EQ(ts2, 1331737200000);

Review Comment:
   How about adding a test for failure case too?



##########
cpp/src/gandiva/precompiled/time.cc:
##########
@@ -1055,4 +1055,5 @@ gdv_int32 datediff_timestamp_timestamp(gdv_timestamp start_millis,
 CAST_NULLABLE_INTERVAL_YEAR(int32)
 CAST_NULLABLE_INTERVAL_YEAR(int64)
 
+

Review Comment:
   ```suggestion
   ```



##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,51 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::locate_zone;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  try {
+    const time_zone* local_tz = locate_zone(std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count()*1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+      gdv_fn_context_set_error_msg(context, "Invalid time zone");
+      return 0;

Review Comment:
   ```suggestion
       gdv_fn_context_set_error_msg(context, "Invalid time zone");
       return 0;
   ```



##########
cpp/src/gandiva/function_registry_datetime.cc:
##########
@@ -163,11 +163,19 @@ std::vector<NativeFunction> GetDateTimeFunctionRegistry() {
       NativeFunction("datediff", {}, DataTypeVector{timestamp(), timestamp()}, int32(),
                      kResultNullIfNull, "datediff_timestamp_timestamp"),
 
+      NativeFunction("to_utc_timestamp", {}, DataTypeVector{timestamp(), utf8()},
+                     timestamp(), kResultNullIfNull, "to_utc_timezone_timestamp",
+                     NativeFunction::kNeedsContext),
+
+      NativeFunction("from_utc_timestamp", {}, DataTypeVector{timestamp(), utf8()},
+                     timestamp(), kResultNullIfNull, "from_utc_timezone_timestamp",
+                     NativeFunction::kNeedsContext),
+
       DATE_TYPES(LAST_DAY_SAFE_NULL_IF_NULL, last_day, {}),
       BASE_NUMERIC_TYPES(TO_TIME_SAFE_NULL_IF_NULL, to_time, {}),
       BASE_NUMERIC_TYPES(TO_TIMESTAMP_SAFE_NULL_IF_NULL, to_timestamp, {})};
 
   return date_time_fn_registry_;
 }
 
-}  // namespace gandiva
+}  // namespace gandiva

Review Comment:
   Could you revert this change before we merge this?



##########
cpp/src/gandiva/precompiled/types.h:
##########
@@ -433,6 +433,7 @@ gdv_time32 castTIME_utf8(int64_t context, const char* input, int32_t length);
 gdv_time32 castTIME_timestamp(gdv_timestamp timestamp_in_millis);
 gdv_time32 castTIME_int32(int32_t int_val);
 const char* castVARCHAR_timestamp_int64(int64_t, gdv_timestamp, gdv_int64, gdv_int32*);
+

Review Comment:
   ```suggestion
   ```



##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,51 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::locate_zone;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  try {
+    const time_zone* local_tz = locate_zone(std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count()*1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+      gdv_fn_context_set_error_msg(context, "Invalid time zone");
+      return 0;
+  }
+}
+
+GANDIVA_EXPORT
+gdv_timestamp from_utc_timezone_timestamp(gdv_int64 context,
+                                          gdv_timestamp time_miliseconds,
+                                          const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::make_zoned;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  const zoned_time<milliseconds, const time_zone*> utc_tz =
+                      make_zoned(std::string("Etc/UTC"), tp);
+  try {
+    const zoned_time<milliseconds, const time_zone*> local_tz =
+                      make_zoned(std::string(timezone, length), utc_tz);
+    gdv_timestamp offset = local_tz.get_time_zone()->get_info(tp).offset.count()*1000;
+    return time_miliseconds + static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+      gdv_fn_context_set_error_msg(context, "Invalid time zone");

Review Comment:
   How about adding the given `timezone` to error message? It will help users to fix the invalid input.



##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,51 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::locate_zone;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  try {
+    const time_zone* local_tz = locate_zone(std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count()*1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+      gdv_fn_context_set_error_msg(context, "Invalid time zone");
+      return 0;
+  }
+}
+
+GANDIVA_EXPORT
+gdv_timestamp from_utc_timezone_timestamp(gdv_int64 context,
+                                          gdv_timestamp time_miliseconds,
+                                          const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::make_zoned;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  const zoned_time<milliseconds, const time_zone*> utc_tz =

Review Comment:
   Can we use `const auto utc_tz = ...` here?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou merged pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
kou merged PR #13428:
URL: https://github.com/apache/arrow/pull/13428


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou commented on a diff in pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
kou commented on code in PR #13428:
URL: https://github.com/apache/arrow/pull/13428#discussion_r919736246


##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,51 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::locate_zone;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  try {
+    const time_zone* local_tz = locate_zone(std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count()*1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+      gdv_fn_context_set_error_msg(context, "Invalid time zone");
+      return 0;
+  }
+}
+
+GANDIVA_EXPORT
+gdv_timestamp from_utc_timezone_timestamp(gdv_int64 context,
+                                          gdv_timestamp time_miliseconds,
+                                          const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::make_zoned;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  const zoned_time<milliseconds, const time_zone*> utc_tz =

Review Comment:
   OK. Is this the error log? https://ci.appveyor.com/project/ApacheSoftwareFoundation/arrow/builds/44007267/job/2upjodye0ip00r66
   And is this the fix of the error? https://github.com/palak-9202/arrow/commit/4c0549526e56866205e4b1baa993b2e3e67241d5



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] palak-9202 commented on a diff in pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
palak-9202 commented on code in PR #13428:
URL: https://github.com/apache/arrow/pull/13428#discussion_r918607114


##########
cpp/src/gandiva/gdv_function_stubs_test.cc:
##########
@@ -993,4 +993,57 @@ TEST(TestGdvFnStubs, TestTranslate) {
   EXPECT_EQ(expected, std::string(result, out_len));
 }
 
+TEST(TestTime, TestToUtcTimezone) {
+  ExecutionContext context;
+  auto context_ptr = reinterpret_cast<int64_t>(&context);
+  gdv_int32 len_ist = static_cast<gdv_int32>(strlen("Asia/Kolkata"));
+  gdv_int32 len_pst = static_cast<gdv_int32>(strlen("America/Los_Angeles"));
+
+  //2012-02-28 15:30:00 Asia/Kolkata
+  gdv_timestamp ts = 55800000;
+  gdv_timestamp ts2 = to_utc_timezone_timestamp(context_ptr, ts,
+                                                "Asia/Kolkata", len_ist);
+  EXPECT_EQ(36000000, ts2);
+
+  //1970-01-01 5:00:00 Asia/Kolkata
+  ts = 18000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "Asia/Kolkata", len_ist);
+  EXPECT_EQ(ts2, -1800000);
+
+  //daylight savings check
+  //2018-03-11 01:00:00 America/Los_Angeles
+  ts = 	1520730000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "America/Los_Angeles", len_pst);
+  EXPECT_EQ(ts2, 1520758800000);
+
+  //2018-03-12 01:00:00 America/Los_Angeles
+  ts = 1331712000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "America/Los_Angeles", len_pst);
+  EXPECT_EQ(ts2, 1331737200000);

Review Comment:
   I've added a failure test and modified the error message to include the time zone name given as input



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou commented on pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
kou commented on PR #13428:
URL: https://github.com/apache/arrow/pull/13428#issuecomment-1179462745

   We can fix style by `cmake --build ${BUILD_DIR} --target format` but you need to install `clang-format-12`.
   If it's difficult to prepare `clang-format-12`, I can fix style and push the fix to this branch.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou commented on a diff in pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
kou commented on code in PR #13428:
URL: https://github.com/apache/arrow/pull/13428#discussion_r919659653


##########
cpp/src/gandiva/gdv_function_stubs_test.cc:
##########
@@ -993,4 +993,61 @@ TEST(TestGdvFnStubs, TestTranslate) {
   EXPECT_EQ(expected, std::string(result, out_len));
 }
 
+TEST(TestGdvFnStubs, TestToUtcTimezone) {
+  gandiva::ExecutionContext context;
+  auto context_ptr = reinterpret_cast<int64_t>(&context);
+  gdv_int32 len_ist = static_cast<gdv_int32>(strlen("Asia/Kolkata"));
+  gdv_int32 len_pst = static_cast<gdv_int32>(strlen("America/Los_Angeles"));
+
+  //2012-02-28 15:30:00 Asia/Kolkata

Review Comment:
   Is this correct?



##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,67 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::locate_zone;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  try {
+    const time_zone* local_tz = locate_zone(std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count()*1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+    int32_t msg_len = static_cast<int32_t>(strlen(timezone) + 50);
+    char* err_msg = reinterpret_cast<char*>(gdv_fn_context_arena_malloc(context,
+                                            msg_len));
+    if (err_msg == nullptr) {
+      gdv_fn_context_set_error_msg(context, "Could not allocate memory");
+      return 0;
+    }
+    std::snprintf(err_msg, msg_len, "'%s' is an invalid time zone name.", timezone);
+    gdv_fn_context_set_error_msg(context, err_msg);

Review Comment:
   @anthonylouisbsb Could you confirm whether this is a preferred error message generation approach in Gandiva? (I'm not familiar with Gandiva's error message generation.)



##########
cpp/src/gandiva/gdv_function_stubs_test.cc:
##########
@@ -993,4 +993,61 @@ TEST(TestGdvFnStubs, TestTranslate) {
   EXPECT_EQ(expected, std::string(result, out_len));
 }
 
+TEST(TestGdvFnStubs, TestToUtcTimezone) {
+  gandiva::ExecutionContext context;
+  auto context_ptr = reinterpret_cast<int64_t>(&context);
+  gdv_int32 len_ist = static_cast<gdv_int32>(strlen("Asia/Kolkata"));
+  gdv_int32 len_pst = static_cast<gdv_int32>(strlen("America/Los_Angeles"));
+
+  //2012-02-28 15:30:00 Asia/Kolkata
+  gdv_timestamp ts = 55800000;
+  gdv_timestamp ts2 = to_utc_timezone_timestamp(context_ptr, ts,
+                                                "Asia/Kolkata", len_ist);
+  EXPECT_EQ(36000000, ts2);
+
+  //1970-01-01 5:00:00 Asia/Kolkata
+  ts = 18000000;

Review Comment:
   Is this correct?
   It seems that `18000000` is "1970-01-01 5:00:00 UTC" and "1970-01-01 5:00:00 Asia/Kolkata" is `-1800000`.



##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,51 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::locate_zone;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  try {
+    const time_zone* local_tz = locate_zone(std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count()*1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+      gdv_fn_context_set_error_msg(context, "Invalid time zone");
+      return 0;
+  }
+}
+
+GANDIVA_EXPORT
+gdv_timestamp from_utc_timezone_timestamp(gdv_int64 context,
+                                          gdv_timestamp time_miliseconds,
+                                          const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::make_zoned;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  const zoned_time<milliseconds, const time_zone*> utc_tz =

Review Comment:
   Umm, I couldn't reproduce it with
   
   ```diff
   diff --git a/cpp/src/gandiva/gdv_function_stubs.cc b/cpp/src/gandiva/gdv_function_stubs.cc
   index ccf3e7ca3b..e6cbeb8aa7 100644
   --- a/cpp/src/gandiva/gdv_function_stubs.cc
   +++ b/cpp/src/gandiva/gdv_function_stubs.cc
   @@ -651,11 +651,9 @@ gdv_timestamp from_utc_timezone_timestamp(gdv_int64 context,
      using arrow_vendored::date::make_zoned;
    
      sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
   -  const zoned_time<milliseconds, const time_zone*> utc_tz =
   -                      make_zoned(std::string("Etc/UTC"), tp);
   +  const auto utc_tz = make_zoned(std::string("Etc/UTC"), tp);
      try {
   -    const zoned_time<milliseconds, const time_zone*> local_tz =
   -                      make_zoned(std::string(timezone, length), utc_tz);
   +    const auto local_tz = make_zoned(std::string(timezone, length), utc_tz);
        gdv_timestamp offset = local_tz.get_time_zone()->get_info(tp).offset.count()*1000;
        return time_miliseconds + static_cast<gdv_timestamp>(offset);
      } catch(...) {
   ```
   
   Could you show an error message on your environment?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] palak-9202 commented on a diff in pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
palak-9202 commented on code in PR #13428:
URL: https://github.com/apache/arrow/pull/13428#discussion_r919915616


##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,51 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::locate_zone;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  try {
+    const time_zone* local_tz = locate_zone(std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count()*1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+      gdv_fn_context_set_error_msg(context, "Invalid time zone");
+      return 0;
+  }
+}
+
+GANDIVA_EXPORT
+gdv_timestamp from_utc_timezone_timestamp(gdv_int64 context,
+                                          gdv_timestamp time_miliseconds,
+                                          const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::make_zoned;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  const zoned_time<milliseconds, const time_zone*> utc_tz =

Review Comment:
   It worked out 😸 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] github-actions[bot] commented on pull request #13428: ARROW-16918: [Gandiva] [C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13428:
URL: https://github.com/apache/arrow/pull/13428#issuecomment-1170951518

   https://issues.apache.org/jira/browse/ARROW-16918


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] palak-9202 commented on a diff in pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
palak-9202 commented on code in PR #13428:
URL: https://github.com/apache/arrow/pull/13428#discussion_r917534687


##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,51 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::locate_zone;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  try {
+    const time_zone* local_tz = locate_zone(std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count()*1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+      gdv_fn_context_set_error_msg(context, "Invalid time zone");
+      return 0;
+  }
+}
+
+GANDIVA_EXPORT
+gdv_timestamp from_utc_timezone_timestamp(gdv_int64 context,
+                                          gdv_timestamp time_miliseconds,
+                                          const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::make_zoned;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  const zoned_time<milliseconds, const time_zone*> utc_tz =

Review Comment:
   I had used 'auto' earlier but this caused a build error because this object is passed to an overloaded function, so the type had to be explicitly specified



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou commented on a diff in pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
kou commented on code in PR #13428:
URL: https://github.com/apache/arrow/pull/13428#discussion_r920615543


##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,65 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::locate_zone;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  try {
+    const auto local_tz = locate_zone(std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count()*1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+    int32_t msg_len = static_cast<int32_t>(strlen(timezone) + 50);
+    char* err_msg =
+        reinterpret_cast<char*>(gdv_fn_context_arena_malloc(context, msg_len));
+    if (err_msg == nullptr) {
+      gdv_fn_context_set_error_msg(context, "Could not allocate memory");
+      return 0;
+    }
+    std::snprintf(err_msg, msg_len, "'%s' is an invalid time zone name.", timezone);
+    gdv_fn_context_set_error_msg(context, err_msg);
+    return 0;
+  }
+}
+
+GANDIVA_EXPORT
+gdv_timestamp from_utc_timezone_timestamp(gdv_int64 context,
+                                          gdv_timestamp time_miliseconds,
+                                          const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;

Review Comment:
   ```suggestion
   ```



##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,65 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::locate_zone;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  try {
+    const auto local_tz = locate_zone(std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count()*1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+    int32_t msg_len = static_cast<int32_t>(strlen(timezone) + 50);

Review Comment:
   ```suggestion
       auto msg_len = static_cast<int32_t>(strlen(timezone) + 50);
   ```



##########
cpp/src/gandiva/gdv_function_stubs_test.cc:
##########
@@ -993,4 +993,61 @@ TEST(TestGdvFnStubs, TestTranslate) {
   EXPECT_EQ(expected, std::string(result, out_len));
 }
 
+TEST(TestGdvFnStubs, TestToUtcTimezone) {
+  gandiva::ExecutionContext context;
+  auto context_ptr = reinterpret_cast<int64_t>(&context);
+  gdv_int32 len_ist = static_cast<gdv_int32>(strlen("Asia/Kolkata"));
+  gdv_int32 len_pst = static_cast<gdv_int32>(strlen("America/Los_Angeles"));
+
+  //2012-02-28 15:30:00
+  gdv_timestamp ts = 1330443000000;
+  gdv_timestamp ts2 =
+      to_utc_timezone_timestamp(context_ptr, ts, "Asia/Kolkata", len_ist);
+  EXPECT_EQ(1330423200000, ts2);
+
+  //1970-01-01 5:00:00
+  ts = 18000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "Asia/Kolkata", len_ist);
+  EXPECT_EQ(ts2, -1800000);
+
+  //daylight savings check
+  //2018-03-11 01:00:00
+  ts = 	1520730000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "America/Los_Angeles", len_pst);
+  EXPECT_EQ(ts2, 1520758800000);
+
+  //2018-03-12 01:00:00
+  ts = 1331712000000;

Review Comment:
   Is this correct?
   It seems that 1331712000000 is "2012-03-14 08:00:00 UTC".



##########
cpp/src/gandiva/gdv_function_stubs_test.cc:
##########
@@ -993,4 +993,61 @@ TEST(TestGdvFnStubs, TestTranslate) {
   EXPECT_EQ(expected, std::string(result, out_len));
 }
 
+TEST(TestGdvFnStubs, TestToUtcTimezone) {
+  gandiva::ExecutionContext context;
+  auto context_ptr = reinterpret_cast<int64_t>(&context);
+  gdv_int32 len_ist = static_cast<gdv_int32>(strlen("Asia/Kolkata"));
+  gdv_int32 len_pst = static_cast<gdv_int32>(strlen("America/Los_Angeles"));
+
+  //2012-02-28 15:30:00
+  gdv_timestamp ts = 1330443000000;
+  gdv_timestamp ts2 =
+      to_utc_timezone_timestamp(context_ptr, ts, "Asia/Kolkata", len_ist);
+  EXPECT_EQ(1330423200000, ts2);
+
+  //1970-01-01 5:00:00
+  ts = 18000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "Asia/Kolkata", len_ist);
+  EXPECT_EQ(ts2, -1800000);
+
+  //daylight savings check
+  //2018-03-11 01:00:00
+  ts = 	1520730000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "America/Los_Angeles", len_pst);
+  EXPECT_EQ(ts2, 1520758800000);
+
+  //2018-03-12 01:00:00
+  ts = 1331712000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "America/Los_Angeles", len_pst);
+  EXPECT_EQ(ts2, 1331737200000);
+
+  //Failure case
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "America/LA", 10);
+  EXPECT_THAT(context.get_error(), "'America/LA' is an invalid time zone name.");
+}
+
+TEST(TestGdvFnStubs, TestFromUtcTimezone) {
+  ExecutionContext context;
+  auto context_ptr = reinterpret_cast<int64_t>(&context);
+  gdv_int32 len_ist = static_cast<gdv_int32>(strlen("Asia/Kolkata"));
+  gdv_int32 len_pst = static_cast<gdv_int32>(strlen("America/Los_Angeles"));

Review Comment:
   ```suggestion
     auto len_ist = static_cast<gdv_int32>(strlen("Asia/Kolkata"));
     auto len_pst = static_cast<gdv_int32>(strlen("America/Los_Angeles"));
   ```



##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,65 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;

Review Comment:
   ```suggestion
   ```



##########
cpp/src/gandiva/gdv_function_stubs_test.cc:
##########
@@ -993,4 +993,61 @@ TEST(TestGdvFnStubs, TestTranslate) {
   EXPECT_EQ(expected, std::string(result, out_len));
 }
 
+TEST(TestGdvFnStubs, TestToUtcTimezone) {
+  gandiva::ExecutionContext context;
+  auto context_ptr = reinterpret_cast<int64_t>(&context);
+  gdv_int32 len_ist = static_cast<gdv_int32>(strlen("Asia/Kolkata"));
+  gdv_int32 len_pst = static_cast<gdv_int32>(strlen("America/Los_Angeles"));

Review Comment:
   ```suggestion
     auto len_ist = static_cast<gdv_int32>(strlen("Asia/Kolkata"));
     auto len_pst = static_cast<gdv_int32>(strlen("America/Los_Angeles"));
   ```



##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,65 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::locate_zone;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  try {
+    const auto local_tz = locate_zone(std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count()*1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+    int32_t msg_len = static_cast<int32_t>(strlen(timezone) + 50);
+    char* err_msg =

Review Comment:
   ```suggestion
       auto err_msg =
   ```



##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,65 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::locate_zone;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  try {
+    const auto local_tz = locate_zone(std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count()*1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+    int32_t msg_len = static_cast<int32_t>(strlen(timezone) + 50);
+    char* err_msg =
+        reinterpret_cast<char*>(gdv_fn_context_arena_malloc(context, msg_len));
+    if (err_msg == nullptr) {
+      gdv_fn_context_set_error_msg(context, "Could not allocate memory");
+      return 0;
+    }
+    std::snprintf(err_msg, msg_len, "'%s' is an invalid time zone name.", timezone);
+    gdv_fn_context_set_error_msg(context, err_msg);
+    return 0;
+  }
+}
+
+GANDIVA_EXPORT
+gdv_timestamp from_utc_timezone_timestamp(gdv_int64 context,
+                                          gdv_timestamp time_miliseconds,
+                                          const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::make_zoned;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  const auto utc_tz = make_zoned(std::string("Etc/UTC"), tp);
+  try {
+    const auto local_tz = make_zoned(std::string(timezone, length), utc_tz);
+    gdv_timestamp offset = local_tz.get_time_zone()->get_info(tp).offset.count()*1000;
+    return time_miliseconds + static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+    int32_t msg_len = static_cast<int32_t>(strlen(timezone) + 50);

Review Comment:
   ```suggestion
       auto msg_len = static_cast<int32_t>(strlen(timezone) + 50);
   ```



##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,65 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::locate_zone;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  try {
+    const auto local_tz = locate_zone(std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count()*1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+    int32_t msg_len = static_cast<int32_t>(strlen(timezone) + 50);
+    char* err_msg =
+        reinterpret_cast<char*>(gdv_fn_context_arena_malloc(context, msg_len));
+    if (err_msg == nullptr) {
+      gdv_fn_context_set_error_msg(context, "Could not allocate memory");
+      return 0;
+    }
+    std::snprintf(err_msg, msg_len, "'%s' is an invalid time zone name.", timezone);
+    gdv_fn_context_set_error_msg(context, err_msg);
+    return 0;
+  }
+}
+
+GANDIVA_EXPORT
+gdv_timestamp from_utc_timezone_timestamp(gdv_int64 context,
+                                          gdv_timestamp time_miliseconds,
+                                          const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::make_zoned;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  const auto utc_tz = make_zoned(std::string("Etc/UTC"), tp);
+  try {
+    const auto local_tz = make_zoned(std::string(timezone, length), utc_tz);
+    gdv_timestamp offset = local_tz.get_time_zone()->get_info(tp).offset.count()*1000;
+    return time_miliseconds + static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+    int32_t msg_len = static_cast<int32_t>(strlen(timezone) + 50);
+    char* err_msg =

Review Comment:
   ```suggestion
       auto err_msg =
   ```



##########
cpp/src/gandiva/gdv_function_stubs_test.cc:
##########
@@ -993,4 +993,61 @@ TEST(TestGdvFnStubs, TestTranslate) {
   EXPECT_EQ(expected, std::string(result, out_len));
 }
 
+TEST(TestGdvFnStubs, TestToUtcTimezone) {
+  gandiva::ExecutionContext context;
+  auto context_ptr = reinterpret_cast<int64_t>(&context);
+  gdv_int32 len_ist = static_cast<gdv_int32>(strlen("Asia/Kolkata"));
+  gdv_int32 len_pst = static_cast<gdv_int32>(strlen("America/Los_Angeles"));
+
+  //2012-02-28 15:30:00
+  gdv_timestamp ts = 1330443000000;
+  gdv_timestamp ts2 =
+      to_utc_timezone_timestamp(context_ptr, ts, "Asia/Kolkata", len_ist);
+  EXPECT_EQ(1330423200000, ts2);
+
+  //1970-01-01 5:00:00
+  ts = 18000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "Asia/Kolkata", len_ist);
+  EXPECT_EQ(ts2, -1800000);
+
+  //daylight savings check
+  //2018-03-11 01:00:00
+  ts = 	1520730000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "America/Los_Angeles", len_pst);
+  EXPECT_EQ(ts2, 1520758800000);
+
+  //2018-03-12 01:00:00
+  ts = 1331712000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "America/Los_Angeles", len_pst);
+  EXPECT_EQ(ts2, 1331737200000);
+
+  //Failure case
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "America/LA", 10);
+  EXPECT_THAT(context.get_error(), "'America/LA' is an invalid time zone name.");
+}
+
+TEST(TestGdvFnStubs, TestFromUtcTimezone) {
+  ExecutionContext context;
+  auto context_ptr = reinterpret_cast<int64_t>(&context);
+  gdv_int32 len_ist = static_cast<gdv_int32>(strlen("Asia/Kolkata"));
+  gdv_int32 len_pst = static_cast<gdv_int32>(strlen("America/Los_Angeles"));
+
+  gdv_timestamp ts = 36000000;
+  gdv_timestamp ts2 =
+      from_utc_timezone_timestamp(context_ptr, ts, "Asia/Kolkata", len_ist);
+  EXPECT_EQ(ts2, 55800000);
+
+  ts = -1800000;
+  ts2 = from_utc_timezone_timestamp(context_ptr, ts, "Asia/Kolkata", len_ist);
+  EXPECT_EQ(ts2, 18000000);
+
+  ts = 1520758800000;
+  ts2 = from_utc_timezone_timestamp(context_ptr, ts, "America/Los_Angeles", len_pst);
+  EXPECT_EQ(ts2, 1520730000000);
+
+  ts = 1331737200000;
+  ts2 = from_utc_timezone_timestamp(context_ptr, ts, "America/Los_Angeles", len_pst);
+  EXPECT_EQ(ts2, 1331712000000);
+}

Review Comment:
   How about adding error case for `from_utc_timezone_timestamp()` too?



##########
cpp/src/gandiva/gdv_function_stubs_test.cc:
##########
@@ -993,4 +993,61 @@ TEST(TestGdvFnStubs, TestTranslate) {
   EXPECT_EQ(expected, std::string(result, out_len));
 }
 
+TEST(TestGdvFnStubs, TestToUtcTimezone) {
+  gandiva::ExecutionContext context;
+  auto context_ptr = reinterpret_cast<int64_t>(&context);
+  gdv_int32 len_ist = static_cast<gdv_int32>(strlen("Asia/Kolkata"));
+  gdv_int32 len_pst = static_cast<gdv_int32>(strlen("America/Los_Angeles"));
+
+  //2012-02-28 15:30:00
+  gdv_timestamp ts = 1330443000000;
+  gdv_timestamp ts2 =
+      to_utc_timezone_timestamp(context_ptr, ts, "Asia/Kolkata", len_ist);
+  EXPECT_EQ(1330423200000, ts2);
+
+  //1970-01-01 5:00:00
+  ts = 18000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "Asia/Kolkata", len_ist);
+  EXPECT_EQ(ts2, -1800000);
+
+  //daylight savings check
+  //2018-03-11 01:00:00
+  ts = 	1520730000000;

Review Comment:
   ```suggestion
     ts = 1520730000000;
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou commented on pull request #13428: MINOR: [Gandiva] [C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
kou commented on PR #13428:
URL: https://github.com/apache/arrow/pull/13428#issuecomment-1165545029

   Could you open a JIRA issue because this is not a minor fix https://github.com/apache/arrow/blob/master/CONTRIBUTING.md#Minor-Fixes ?
   https://issues.apache.org/jira/browse/ARROW


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] palak-9202 commented on a diff in pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
palak-9202 commented on code in PR #13428:
URL: https://github.com/apache/arrow/pull/13428#discussion_r920799751


##########
cpp/src/gandiva/gdv_function_stubs_test.cc:
##########
@@ -993,4 +993,61 @@ TEST(TestGdvFnStubs, TestTranslate) {
   EXPECT_EQ(expected, std::string(result, out_len));
 }
 
+TEST(TestGdvFnStubs, TestToUtcTimezone) {
+  gandiva::ExecutionContext context;
+  auto context_ptr = reinterpret_cast<int64_t>(&context);
+  gdv_int32 len_ist = static_cast<gdv_int32>(strlen("Asia/Kolkata"));
+  gdv_int32 len_pst = static_cast<gdv_int32>(strlen("America/Los_Angeles"));
+
+  //2012-02-28 15:30:00
+  gdv_timestamp ts = 1330443000000;
+  gdv_timestamp ts2 =
+      to_utc_timezone_timestamp(context_ptr, ts, "Asia/Kolkata", len_ist);
+  EXPECT_EQ(1330423200000, ts2);
+
+  //1970-01-01 5:00:00
+  ts = 18000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "Asia/Kolkata", len_ist);
+  EXPECT_EQ(ts2, -1800000);
+
+  //daylight savings check
+  //2018-03-11 01:00:00
+  ts = 	1520730000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "America/Los_Angeles", len_pst);
+  EXPECT_EQ(ts2, 1520758800000);
+
+  //2018-03-12 01:00:00
+  ts = 1331712000000;
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "America/Los_Angeles", len_pst);
+  EXPECT_EQ(ts2, 1331737200000);
+
+  //Failure case
+  ts2 = to_utc_timezone_timestamp(context_ptr, ts, "America/LA", 10);
+  EXPECT_THAT(context.get_error(), "'America/LA' is an invalid time zone name.");
+}
+
+TEST(TestGdvFnStubs, TestFromUtcTimezone) {
+  ExecutionContext context;
+  auto context_ptr = reinterpret_cast<int64_t>(&context);
+  gdv_int32 len_ist = static_cast<gdv_int32>(strlen("Asia/Kolkata"));
+  gdv_int32 len_pst = static_cast<gdv_int32>(strlen("America/Los_Angeles"));
+
+  gdv_timestamp ts = 36000000;
+  gdv_timestamp ts2 =
+      from_utc_timezone_timestamp(context_ptr, ts, "Asia/Kolkata", len_ist);
+  EXPECT_EQ(ts2, 55800000);
+
+  ts = -1800000;
+  ts2 = from_utc_timezone_timestamp(context_ptr, ts, "Asia/Kolkata", len_ist);
+  EXPECT_EQ(ts2, 18000000);
+
+  ts = 1520758800000;
+  ts2 = from_utc_timezone_timestamp(context_ptr, ts, "America/Los_Angeles", len_pst);
+  EXPECT_EQ(ts2, 1520730000000);
+
+  ts = 1331737200000;
+  ts2 = from_utc_timezone_timestamp(context_ptr, ts, "America/Los_Angeles", len_pst);
+  EXPECT_EQ(ts2, 1331712000000);
+}

Review Comment:
   I've added all the suggested changes



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] palak-9202 commented on a diff in pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
palak-9202 commented on code in PR #13428:
URL: https://github.com/apache/arrow/pull/13428#discussion_r919698275


##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,51 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::locate_zone;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  try {
+    const time_zone* local_tz = locate_zone(std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count()*1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+      gdv_fn_context_set_error_msg(context, "Invalid time zone");
+      return 0;
+  }
+}
+
+GANDIVA_EXPORT
+gdv_timestamp from_utc_timezone_timestamp(gdv_int64 context,
+                                          gdv_timestamp time_miliseconds,
+                                          const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::make_zoned;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  const zoned_time<milliseconds, const time_zone*> utc_tz =

Review Comment:
   I didn't get any errors on my environment either, the build error occurred in the Appveyor build check



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] github-actions[bot] commented on pull request #13428: adding utc <-> timezone conversion functions

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13428:
URL: https://github.com/apache/arrow/pull/13428#issuecomment-1164491521

   <!--
     Licensed to the Apache Software Foundation (ASF) under one
     or more contributor license agreements.  See the NOTICE file
     distributed with this work for additional information
     regarding copyright ownership.  The ASF licenses this file
     to you under the Apache License, Version 2.0 (the
     "License"); you may not use this file except in compliance
     with the License.  You may obtain a copy of the License at
   
       http://www.apache.org/licenses/LICENSE-2.0
   
     Unless required by applicable law or agreed to in writing,
     software distributed under the License is distributed on an
     "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
     KIND, either express or implied.  See the License for the
     specific language governing permissions and limitations
     under the License.
   -->
   
   Thanks for opening a pull request!
   
   If this is not a [minor PR](https://github.com/apache/arrow/blob/master/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW
   
   Opening JIRAs ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project.
   
   Then could you also rename pull request title in the following format?
   
       ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}
   
   or
   
       MINOR: [${COMPONENT}] ${SUMMARY}
   
   See also:
   
     * [Other pull requests](https://github.com/apache/arrow/pulls/)
     * [Contribution Guidelines - How to contribute patches](https://arrow.apache.org/docs/developers/contributing.html#how-to-contribute-patches)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] palak-9202 commented on a diff in pull request #13428: ARROW-16918: [Gandiva] [C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
palak-9202 commented on code in PR #13428:
URL: https://github.com/apache/arrow/pull/13428#discussion_r914448375


##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,47 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using arrow_vendored::date::time_zone;
+
+  arrow_vendored::date::sys_time <std::chrono::milliseconds> tp
+                                  {std::chrono::milliseconds{time_miliseconds}};
+  try {
+    const time_zone* local_tz = arrow_vendored::date::locate_zone
+                                (std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count()*1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+      gdv_fn_context_set_error_msg(context, "Invalid time zone");

Review Comment:
   Try:
   for(auto const& z : get_tzdb().zones) {
       cout<<z.name()<<'\n';
     }



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] github-actions[bot] commented on pull request #13428: ARROW-16918: [Gandiva] [C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13428:
URL: https://github.com/apache/arrow/pull/13428#issuecomment-1170951560

   :warning: Ticket **has not been started in JIRA**, please click 'Start Progress'.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] palak-9202 commented on a diff in pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
palak-9202 commented on code in PR #13428:
URL: https://github.com/apache/arrow/pull/13428#discussion_r919694197


##########
cpp/src/gandiva/gdv_function_stubs_test.cc:
##########
@@ -993,4 +993,61 @@ TEST(TestGdvFnStubs, TestTranslate) {
   EXPECT_EQ(expected, std::string(result, out_len));
 }
 
+TEST(TestGdvFnStubs, TestToUtcTimezone) {
+  gandiva::ExecutionContext context;
+  auto context_ptr = reinterpret_cast<int64_t>(&context);
+  gdv_int32 len_ist = static_cast<gdv_int32>(strlen("Asia/Kolkata"));
+  gdv_int32 len_pst = static_cast<gdv_int32>(strlen("America/Los_Angeles"));
+
+  //2012-02-28 15:30:00 Asia/Kolkata
+  gdv_timestamp ts = 55800000;
+  gdv_timestamp ts2 = to_utc_timezone_timestamp(context_ptr, ts,
+                                                "Asia/Kolkata", len_ist);
+  EXPECT_EQ(36000000, ts2);
+
+  //1970-01-01 5:00:00 Asia/Kolkata
+  ts = 18000000;

Review Comment:
   You're right, I'll make the changes for both these cases



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] palak-9202 commented on a diff in pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
palak-9202 commented on code in PR #13428:
URL: https://github.com/apache/arrow/pull/13428#discussion_r919760316


##########
cpp/src/gandiva/gdv_function_stubs_test.cc:
##########
@@ -993,4 +993,61 @@ TEST(TestGdvFnStubs, TestTranslate) {
   EXPECT_EQ(expected, std::string(result, out_len));
 }
 
+TEST(TestGdvFnStubs, TestToUtcTimezone) {
+  gandiva::ExecutionContext context;
+  auto context_ptr = reinterpret_cast<int64_t>(&context);
+  gdv_int32 len_ist = static_cast<gdv_int32>(strlen("Asia/Kolkata"));
+  gdv_int32 len_pst = static_cast<gdv_int32>(strlen("America/Los_Angeles"));
+
+  //2012-02-28 15:30:00 Asia/Kolkata
+  gdv_timestamp ts = 55800000;
+  gdv_timestamp ts2 = to_utc_timezone_timestamp(context_ptr, ts,
+                                                "Asia/Kolkata", len_ist);
+  EXPECT_EQ(36000000, ts2);
+
+  //1970-01-01 5:00:00 Asia/Kolkata
+  ts = 18000000;

Review Comment:
   Hey, actually since the function takes in the timestamp separate from the time zone, the time stamp is not adjusted for zone offset (this is done to match the hive function implementation). so what the function receives is a timestamp that should be interpreted as the time in the zone given as second parameter. That's why the function subtracts the offset to get UTC time even though the actual equivalent timestamp is the same, just the representation of the date+time is different.
   So my comments are misleading- the input is just a date + time whose timestamp will be casted based on 0 offset. See the projector test about this.
   I will remove the time zone names from the comments.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] palak-9202 commented on a diff in pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
palak-9202 commented on code in PR #13428:
URL: https://github.com/apache/arrow/pull/13428#discussion_r919800982


##########
cpp/src/gandiva/gdv_function_stubs.cc:
##########
@@ -611,6 +611,51 @@ int32_t gdv_fn_cast_intervalyear_utf8_int32(int64_t context_ptr, int64_t holder_
   auto* holder = reinterpret_cast<gandiva::IntervalYearsHolder*>(holder_ptr);
   return (*holder)(context, data, data_len, in1_validity, out_valid);
 }
+
+GANDIVA_EXPORT
+gdv_timestamp to_utc_timezone_timestamp(int64_t context, gdv_timestamp time_miliseconds,
+                                        const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::locate_zone;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  try {
+    const time_zone* local_tz = locate_zone(std::string(timezone, length));
+    gdv_timestamp offset = local_tz->get_info(tp).offset.count()*1000;
+    return time_miliseconds - static_cast<gdv_timestamp>(offset);
+  } catch(...) {
+      gdv_fn_context_set_error_msg(context, "Invalid time zone");
+      return 0;
+  }
+}
+
+GANDIVA_EXPORT
+gdv_timestamp from_utc_timezone_timestamp(gdv_int64 context,
+                                          gdv_timestamp time_miliseconds,
+                                          const char* timezone, gdv_int32 length) {
+  using std::chrono::milliseconds;
+  using arrow_vendored::date::sys_time;
+  using arrow_vendored::date::time_zone;
+  using arrow_vendored::date::zoned_time;
+  using arrow_vendored::date::make_zoned;
+
+  sys_time <milliseconds> tp {milliseconds{time_miliseconds}};
+  const zoned_time<milliseconds, const time_zone*> utc_tz =

Review Comment:
   I'll try it out



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou commented on a diff in pull request #13428: ARROW-16918: [Gandiva][C++] Adding UTC-local timezone conversion functions

Posted by GitBox <gi...@apache.org>.
kou commented on code in PR #13428:
URL: https://github.com/apache/arrow/pull/13428#discussion_r920622543


##########
cpp/src/gandiva/gdv_function_stubs_test.cc:
##########
@@ -993,4 +993,61 @@ TEST(TestGdvFnStubs, TestTranslate) {
   EXPECT_EQ(expected, std::string(result, out_len));
 }
 
+TEST(TestGdvFnStubs, TestToUtcTimezone) {
+  gandiva::ExecutionContext context;
+  auto context_ptr = reinterpret_cast<int64_t>(&context);
+  gdv_int32 len_ist = static_cast<gdv_int32>(strlen("Asia/Kolkata"));
+  gdv_int32 len_pst = static_cast<gdv_int32>(strlen("America/Los_Angeles"));
+
+  //2012-02-28 15:30:00 Asia/Kolkata
+  gdv_timestamp ts = 55800000;
+  gdv_timestamp ts2 = to_utc_timezone_timestamp(context_ptr, ts,
+                                                "Asia/Kolkata", len_ist);
+  EXPECT_EQ(36000000, ts2);
+
+  //1970-01-01 5:00:00 Asia/Kolkata
+  ts = 18000000;

Review Comment:
   I see.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org