SQL: Fix getTime() methods in JDBC (#40484)

Previously, `getTime(colIdx/colLabel)` and
`getObject(colIdx/colLabel, java.sql.Time.class)` methods were computing
the time from a `ZonedDateTime` by applying day in millis modulo on the epoch millis
of the `ZonedDateTime` object. This is wrong as we need to keep the time
related fields at the timezone of the `ZonedDateTime` object and just
set the date info to the epoch date (01/01/1970).

Additionally fixes a testing issue as the original timezone id is converted
to an offset string when parsing the response from the server.

Author: matriv

x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcDateUtils.java

@@ -9,6 +9,7 @@
 import java.sql.Date;
 import java.sql.Time;
 import java.sql.Timestamp;
+import java.time.LocalDate;
 import java.time.ZonedDateTime;
 import java.time.format.DateTimeFormatter;
 import java.time.format.DateTimeFormatterBuilder;
@@ -27,10 +28,9 @@
  */
 final class JdbcDateUtils {
 
-    private JdbcDateUtils() {
-    }
+    private JdbcDateUtils() {}
 
-    private static final long DAY_IN_MILLIS = 60 * 60 * 24 * 1000L;
+    private static final LocalDate EPOCH = LocalDate.of(1970, 1, 1);
 
     static final DateTimeFormatter ISO_WITH_MILLIS = new DateTimeFormatterBuilder()
         .parseCaseInsensitive()
@@ -58,20 +58,9 @@ static Date asDate(String date) {
         return new Date(zdt.toLocalDate().atStartOfDay(zdt.getZone()).toInstant().toEpochMilli());
     }
 
-    /**
-     * In contrast to {@link JdbcDateUtils#asDate(String)} here we just want to eliminate
-     * the date part and just set it to EPOCH (1970-01-1)
-     */
-    static Time asTime(long millisSinceEpoch) {
-        return new Time(utcMillisRemoveDate(millisSinceEpoch));
-    }
-
-    /**
-     * In contrast to {@link JdbcDateUtils#asDate(String)} here we just want to eliminate
-     * the date part and just set it to EPOCH (1970-01-1)
-     */
     static Time asTime(String date) {
-        return asTime(asMillisSinceEpoch(date));
+        ZonedDateTime zdt = asDateTime(date);
+        return new Time(zdt.toLocalTime().atDate(EPOCH).atZone(zdt.getZone()).toInstant().toEpochMilli());
     }
 
     static Timestamp asTimestamp(long millisSinceEpoch) {
@@ -93,8 +82,4 @@ static <R> R asDateTimeField(Object value, Function<String, R> asDateTimeMethod,
             return ctor.apply(((Number) value).longValue());
         }
     }
-
-    private static long utcMillisRemoveDate(long l) {
-        return l % DAY_IN_MILLIS;
-    }
 }

x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java

@@ -23,10 +23,13 @@
 import java.nio.file.Path;
 import java.nio.file.SimpleFileVisitor;
 import java.nio.file.attribute.BasicFileAttributes;
+import java.sql.Date;
 import java.sql.ResultSet;
 import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
+import java.sql.Time;
 import java.time.Instant;
+import java.time.LocalDate;
 import java.time.ZoneId;
 import java.time.ZonedDateTime;
 import java.util.ArrayList;
@@ -37,15 +40,17 @@
 
 import static org.elasticsearch.xpack.sql.action.BasicFormatter.FormatOption.CLI;
 
-public abstract class JdbcTestUtils {
+final class JdbcTestUtils {
 
-    public static final String SQL_TRACE = "org.elasticsearch.xpack.sql:TRACE";
+    private JdbcTestUtils() {}
 
-    public static final String JDBC_TIMEZONE = "timezone";
-    
-    public static ZoneId UTC = ZoneId.of("Z");
+    private static final int MAX_WIDTH = 20;
+
+    static final String SQL_TRACE = "org.elasticsearch.xpack.sql:TRACE";
+    static final String JDBC_TIMEZONE = "timezone";
+    static final LocalDate EPOCH = LocalDate.of(1970, 1, 1);
 
-    public static void logResultSetMetadata(ResultSet rs, Logger logger) throws SQLException {
+    static void logResultSetMetadata(ResultSet rs, Logger logger) throws SQLException {
         ResultSetMetaData metaData = rs.getMetaData();
         // header
         StringBuilder sb = new StringBuilder();
@@ -75,35 +80,24 @@ public static void logResultSetMetadata(ResultSet rs, Logger logger) throws SQLE
         logger.info(sb.toString());
     }
 
-    private static final int MAX_WIDTH = 20;
-
-    public static void logResultSetData(ResultSet rs, Logger log) throws SQLException {
+    static void logResultSetData(ResultSet rs, Logger log) throws SQLException {
         ResultSetMetaData metaData = rs.getMetaData();
-        StringBuilder sb = new StringBuilder();
-        StringBuilder column = new StringBuilder();
 
         int columns = metaData.getColumnCount();
 
         while (rs.next()) {
-            sb.setLength(0);
-            for (int i = 1; i <= columns; i++) {
-                column.setLength(0);
-                if (i > 1) {
-                    sb.append(" | ");
-                }
-                sb.append(trimOrPad(column.append(rs.getString(i))));
-            }
-            log.info(sb);
+            log.info(rowAsString(rs, columns));
         }
     }
 
-    public static String resultSetCurrentData(ResultSet rs) throws SQLException {
+    static String resultSetCurrentData(ResultSet rs) throws SQLException {
         ResultSetMetaData metaData = rs.getMetaData();
-        StringBuilder column = new StringBuilder();
-
-        int columns = metaData.getColumnCount();
+        return rowAsString(rs, metaData.getColumnCount());
+    }
 
+    private static String rowAsString(ResultSet rs, int columns) throws SQLException {
         StringBuilder sb = new StringBuilder();
+        StringBuilder column = new StringBuilder();
         for (int i = 1; i <= columns; i++) {
             column.setLength(0);
             if (i > 1) {
@@ -153,7 +147,7 @@ public static void logLikeCLI(ResultSet rs, Logger logger) throws SQLException {
         logger.info("\n" + formatter.formatWithHeader(cols, data));
     }
 
-    public static String of(long millis, String zoneId) {
+    static String of(long millis, String zoneId) {
         return StringUtils.toString(ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), ZoneId.of(zoneId)));
     }
 
@@ -165,7 +159,7 @@ public static String of(long millis, String zoneId) {
      * folders in the file-system (typically IDEs) or
      * inside jars (gradle).
      */
-    public static List<URL> classpathResources(String pattern) throws Exception {
+    static List<URL> classpathResources(String pattern) throws Exception {
         while (pattern.startsWith("/")) {
             pattern = pattern.substring(1);
         }
@@ -234,4 +228,15 @@ static Tuple<String, String> pathAndName(String string) {
         }
         return new Tuple<>(folder, file);
     }
-}
+
+    static Date asDate(long millis, ZoneId zoneId) {
+        return new java.sql.Date(
+            ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), zoneId)
+                .toLocalDate().atStartOfDay(zoneId).toInstant().toEpochMilli());
+    }
+
+    static Time asTime(long millis, ZoneId zoneId) {
+        return new Time(ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), zoneId)
+                .toLocalTime().atDate(JdbcTestUtils.EPOCH).atZone(zoneId).toInstant().toEpochMilli());
+    }
+}

x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java

@@ -34,7 +34,6 @@
 import java.sql.Types;
 import java.time.Instant;
 import java.time.ZoneId;
-import java.time.ZonedDateTime;
 import java.util.Arrays;
 import java.util.Calendar;
 import java.util.Date;
@@ -61,6 +60,8 @@
 import static java.util.Calendar.SECOND;
 import static java.util.Calendar.YEAR;
 import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.JDBC_TIMEZONE;
+import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.asDate;
+import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.asTime;
 import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.of;
 
 public class ResultSetTestCase extends JdbcIntegrationTestCase {
@@ -880,10 +881,7 @@ public void testGettingDateWithoutCalendar() throws Exception {
         doWithQuery(SELECT_ALL_FIELDS, (results) -> {
             results.next();
 
-            ZoneId zoneId = ZoneId.of(timeZoneId);
-            java.sql.Date expectedDate = new java.sql.Date(
-                ZonedDateTime.ofInstant(Instant.ofEpochMilli(randomLongDate), zoneId)
-                    .toLocalDate().atStartOfDay(zoneId).toInstant().toEpochMilli());
+            java.sql.Date expectedDate = asDate(randomLongDate, getZoneFromOffset(randomLongDate));
 
             assertEquals(expectedDate, results.getDate("test_date"));
             assertEquals(expectedDate, results.getDate(9));
@@ -939,11 +937,11 @@ public void testGettingTimeWithoutCalendar() throws Exception {
         });
         Long randomLongDate = randomNonNegativeLong();
         indexSimpleDocumentWithTrueValues(randomLongDate);
-        
+
         doWithQuery(SELECT_ALL_FIELDS, (results) -> {
             results.next();
 
-            java.sql.Time expectedTime = new java.sql.Time(randomLongDate % 86400000L);
+            java.sql.Time expectedTime = asTime(randomLongDate, getZoneFromOffset(randomLongDate));
 
             assertEquals(expectedTime, results.getTime("test_date"));
             assertEquals(expectedTime, results.getTime(9));
@@ -953,7 +951,7 @@ public void testGettingTimeWithoutCalendar() throws Exception {
             validateErrorsForTimeTestsWithoutCalendar(results::getTime);
         });
     }
-    
+
     public void testGettingTimeWithCalendar() throws Exception {
         createIndex("test");
         updateMappingForNumericValuesTests("test");
@@ -1748,4 +1746,8 @@ private Connection esWithLeniency(boolean multiValueLeniency) throws SQLExceptio
     private String asDateString(long millis) {
         return of(millis, timeZoneId);
     }
+
+    private ZoneId getZoneFromOffset(Long randomLongDate) {
+        return ZoneId.of(ZoneId.of(timeZoneId).getRules().getOffset(Instant.ofEpochMilli(randomLongDate)).toString());
+    }
 }