xtern commented on code in PR #5750:
URL: https://github.com/apache/ignite-3/pull/5750#discussion_r2080972078


##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/exp/IgniteSqlFunctionsTest.java:
##########
@@ -495,4 +505,49 @@ public void testAvgDivide(String a, String b, @Nullable 
String expected) {
             assertThrows(ArithmeticException.class, () -> 
IgniteMath.decimalDivide(num, denum, 4, 2));
         }
     }
+
+    @ParameterizedTest
+    @MethodSource("timeZoneTime")
+    public void toTimestampWithLocalTimeZone(String zoneIdstr, LocalDateTime 
time) {
+        ZoneId zoneId = ZoneId.of(zoneIdstr);
+
+        String v = time.format(DateTimeFormatter.ofPattern("yyyy-MM-dd 
HH:mm:ss.SSS"));
+
+        TimeZone timeZone = TimeZone.getTimeZone(zoneId);
+        long calciteTsLtz = SqlFunctions.toTimestampWithLocalTimeZone(v, 
timeZone);
+        long tsLtz = IgniteSqlFunctions.toTimestampWithLocalTimeZone(v, 
timeZone);
+
+        assertEquals(Instant.ofEpochMilli(calciteTsLtz), 
Instant.ofEpochMilli(tsLtz));
+    }
+
+    @ParameterizedTest
+    @MethodSource("timeZoneTime")
+    public void toTimestampWithLocalTimeZoneFormat(String zoneIdstr, 
LocalDateTime time) {
+        ZoneId zoneId = ZoneId.of(zoneIdstr);
+
+        String v = time.format(DateTimeFormatter.ofPattern("yyyy-MM-dd 
HH:mm:ss.SSS"));
+
+        TimeZone timeZone = TimeZone.getTimeZone(zoneId);
+        long calciteTsLtz = SqlFunctions.toTimestampWithLocalTimeZone(v, 
timeZone);
+        long tsLtz = IgniteSqlFunctions.toTimestampWithLocalTimeZone(v, 
"yyyy-MM-dd hh:mi:ss.ff3", timeZone);
+
+        assertEquals(Instant.ofEpochMilli(calciteTsLtz), 
Instant.ofEpochMilli(tsLtz));
+    }
+
+    private static Stream<Arguments> timeZoneTime() {
+        List<String> zones = List.of("Europe/Paris", "Europe/Moscow", 
"Asia/Tokyo", "America/New_York");

Review Comment:
   Please add some cases that will test the cases with daylight saving time.
   
   See for example: IgniteSqlDateTimeUtilsTest.testSubtractTimeZoneOffset



##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/exp/IgniteSqlFunctions.java:
##########
@@ -543,6 +550,32 @@ public static Object consumeFirstArgument(Object args0, 
Object args1) {
         return args1;
     }
 
+    /** Converts timestamp string into a timestamp with local time zone. */
+    public static @Nullable Long toTimestampWithLocalTimeZone(@Nullable String 
v, TimeZone timeZone) {
+        return SqlFunctions.toTimestampWithLocalTimeZone(v, timeZone);
+    }
+
+    /** Converts timestamp string into a timestamp with local time zone. */
+    public static @Nullable Long toTimestampWithLocalTimeZone(@Nullable String 
v, String format, TimeZone timeZone) {
+        if (v == null) {
+            return null;
+        }
+
+        Objects.requireNonNull(format, "format");
+        Objects.requireNonNull(timeZone, "timeZone");
+
+        // TODO https://issues.apache.org/jira/browse/IGNITE-25320 reuse to 
improve performance.
+        DateParseFunction function = new DateParseFunction();
+        long ts = function.parseTimestamp(format, v);
+        Instant instant = Instant.ofEpochMilli(ts);
+
+        // Adjust instant millis
+        ZoneRules rules = timeZone.toZoneId().getRules();
+        ZoneOffset offset = rules.getOffset(instant);
+        Instant adjusted = instant.minus(offset.getTotalSeconds(), 
ChronoUnit.SECONDS);
+        return adjusted.toEpochMilli();

Review Comment:
   May be this code can be replaced with the following? (needs to be checked 
carefully)
   ```suggestion
       return IgniteSqlDateTimeUtils.subtractTimeZoneOffset(ts, timeZone);
   ```



-- 
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: notifications-unsubscr...@ignite.apache.org

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

Reply via email to