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