SQL: Fix bug with JDBC timezone setting and DATE type (#39978)

Previously, JDBC's REST call to the server was always sending UTC
instead of the timezone passed through connection string/properties.

Moreover the conversion to java.sql.Date was problematic as a
calculation on the epoch millis was used to set the time to 00:00:00.000
and the timezone info was lost. This caused the resulting java.sql.Date
object which is always using the JVM's timezone (no matter what timezone
setting is used in the connection string/properties) to be wrongly created.

Fixes: #39915

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

@@ -158,6 +158,10 @@ class JdbcConfiguration extends ConnectionConfiguration {
         return OPTION_NAMES;
     }
 
+    ZoneId zoneId() {
+        return zoneId;
+    }
+
     public boolean debug() {
         return debug;
     }
@@ -170,10 +174,6 @@ class JdbcConfiguration extends ConnectionConfiguration {
         return zoneId != null ? TimeZone.getTimeZone(zoneId) : null;
     }
 
-    public void timeZone(TimeZone timeZone) {
-        this.zoneId = timeZone != null ? timeZone.toZoneId() : null;
-    }
-
     public static boolean canAccept(String url) {
         return (StringUtils.hasText(url) && url.trim().startsWith(JdbcConfiguration.URL_PREFIX));
     }
@@ -188,4 +188,4 @@ class JdbcConfiguration extends ConnectionConfiguration {
 
         return info.toArray(new DriverPropertyInfo[info.size()]);
     }
-}
+}

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

@@ -26,9 +26,12 @@ import static java.time.temporal.ChronoField.SECOND_OF_MINUTE;
  * from {@code org.elasticsearch.xpack.sql.util.DateUtils} and {@code org.elasticsearch.xpack.sql.proto.StringUtils}.
  */
 final class JdbcDateUtils {
-    
+
-    private static final long DAY_IN_MILLIS = 60 * 60 * 24 * 1000;
+    private JdbcDateUtils() {
-    
+    }
+
+    private static final long DAY_IN_MILLIS = 60 * 60 * 24 * 1000L;
+
     static final DateTimeFormatter ISO_WITH_MILLIS = new DateTimeFormatterBuilder()
         .parseCaseInsensitive()
         .append(ISO_LOCAL_DATE)
@@ -42,24 +45,33 @@ final class JdbcDateUtils {
         .appendOffsetId()
         .toFormatter(Locale.ROOT);
 
+    private static ZonedDateTime asDateTime(String date) {
+        return ISO_WITH_MILLIS.parse(date, ZonedDateTime::from);
+    }
+
     static long asMillisSinceEpoch(String date) {
-        return ISO_WITH_MILLIS.parse(date, ZonedDateTime::from).toInstant().toEpochMilli();
+        return asDateTime(date).toInstant().toEpochMilli();
     }
-    
+
     static Date asDate(String date) {
-        return new Date(utcMillisRemoveTime(asMillisSinceEpoch(date)));
+        ZonedDateTime zdt = asDateTime(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(String date) {
         return new Time(utcMillisRemoveDate(asMillisSinceEpoch(date)));
     }
-    
+
     static Timestamp asTimestamp(String date) {
         return new Timestamp(asMillisSinceEpoch(date));
     }
-    
+
     /*
-     * Handles the value received as parameter, as either String (a ZonedDateTime formatted in ISO 8601 standard with millis) - 
+     * Handles the value received as parameter, as either String (a ZonedDateTime formatted in ISO 8601 standard with millis) -
      * date fields being returned formatted like this. Or a Long value, in case of Histograms.
      */
     static <R> R asDateTimeField(Object value, Function<String, R> asDateTimeMethod, Function<Long, R> ctor) {
@@ -70,10 +82,6 @@ final class JdbcDateUtils {
         }
     }
 
-    static long utcMillisRemoveTime(long l) {
-        return l - (l % DAY_IN_MILLIS);
-    }
-
     private static long utcMillisRemoveDate(long l) {
         return l % DAY_IN_MILLIS;
     }

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

@@ -12,10 +12,9 @@ import org.elasticsearch.xpack.sql.client.Version;
 import org.elasticsearch.xpack.sql.proto.ColumnInfo;
 import org.elasticsearch.xpack.sql.proto.MainResponse;
 import org.elasticsearch.xpack.sql.proto.Mode;
-import org.elasticsearch.xpack.sql.proto.Protocol;
+import org.elasticsearch.xpack.sql.proto.RequestInfo;
 import org.elasticsearch.xpack.sql.proto.SqlQueryRequest;
 import org.elasticsearch.xpack.sql.proto.SqlQueryResponse;
-import org.elasticsearch.xpack.sql.proto.RequestInfo;
 import org.elasticsearch.xpack.sql.proto.SqlTypedParamValue;
 
 import java.sql.SQLException;
@@ -50,7 +49,7 @@ class JdbcHttpClient {
 
     Cursor query(String sql, List<SqlTypedParamValue> params, RequestMeta meta) throws SQLException {
         int fetch = meta.fetchSize() > 0 ? meta.fetchSize() : conCfg.pageSize();
-                SqlQueryRequest sqlRequest = new SqlQueryRequest(sql, params, null, Protocol.TIME_ZONE,
+                SqlQueryRequest sqlRequest = new SqlQueryRequest(sql, params, null, conCfg.zoneId(),
                 fetch,
                 TimeValue.timeValueMillis(meta.timeoutInMs()), TimeValue.timeValueMillis(meta.queryTimeoutInMs()),
                 false, new RequestInfo(Mode.JDBC));
@@ -102,4 +101,4 @@ class JdbcHttpClient {
         }
         return cols;
     }
-}
+}

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

@@ -35,7 +35,6 @@ import java.util.function.Function;
 import static java.lang.String.format;
 import static org.elasticsearch.xpack.sql.jdbc.JdbcDateUtils.asDateTimeField;
 import static org.elasticsearch.xpack.sql.jdbc.JdbcDateUtils.asMillisSinceEpoch;
-import static org.elasticsearch.xpack.sql.jdbc.JdbcDateUtils.utcMillisRemoveTime;
 
 class JdbcResultSet implements ResultSet, JdbcWrapper {
 
@@ -258,7 +257,7 @@ class JdbcResultSet implements ResultSet, JdbcWrapper {
                 return asDateTimeField(val, JdbcDateUtils::asMillisSinceEpoch, Function.identity());
             }
             if (EsType.DATE == type) {
-                return utcMillisRemoveTime(asMillisSinceEpoch(val.toString()));
+                return asMillisSinceEpoch(val.toString());
             }
             return val == null ? null : (Long) val;
         } catch (ClassCastException cce) {

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

@@ -0,0 +1,21 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+package org.elasticsearch.xpack.sql.jdbc;
+
+import java.time.Clock;
+import java.time.Duration;
+import java.time.ZoneId;
+import java.time.ZonedDateTime;
+
+final class JdbcTestUtils {
+
+    private JdbcTestUtils() {}
+
+    static ZonedDateTime nowWithMillisResolution(ZoneId zoneId) {
+        Clock millisResolutionClock = Clock.tick(Clock.system(zoneId), Duration.ofMillis(1));
+        return ZonedDateTime.now(millisResolutionClock);
+    }
+}

x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/ColumnInfoTests.java

@@ -6,8 +6,6 @@
 package org.elasticsearch.xpack.sql.jdbc;
 
 import org.elasticsearch.test.ESTestCase;
-import org.elasticsearch.xpack.sql.jdbc.EsType;
-import org.elasticsearch.xpack.sql.jdbc.JdbcColumnInfo;
 
 import static org.elasticsearch.xpack.sql.client.StringUtils.EMPTY;
 

x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcDatabaseMetaDataTests.java

@@ -7,7 +7,6 @@
 package org.elasticsearch.xpack.sql.jdbc;
 
 import org.elasticsearch.test.ESTestCase;
-import org.elasticsearch.xpack.sql.jdbc.JdbcDatabaseMetaData;
 
 public class JdbcDatabaseMetaDataTests extends ESTestCase {
 
@@ -17,6 +16,5 @@ public class JdbcDatabaseMetaDataTests extends ESTestCase {
         assertEquals(":", md.getCatalogSeparator());
         assertEquals("\"", md.getIdentifierQuoteString());
         assertEquals("\\", md.getSearchStringEscape());
-        
     }
 }

x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/SqlQueryParameterAnalyzerTests.java

@@ -6,7 +6,6 @@
 package org.elasticsearch.xpack.sql.jdbc;
 
 import org.elasticsearch.test.ESTestCase;
-import org.elasticsearch.xpack.sql.jdbc.SqlQueryParameterAnalyzer;
 
 import java.sql.SQLException;
 
@@ -54,7 +53,7 @@ public class SqlQueryParameterAnalyzerTests extends ESTestCase {
         assertEquals("Cannot parse given sql; unclosed /* comment", exception.getMessage());
     }
 
-    public void testUnclosedSingleQuoteStrign() {
+    public void testUnclosedSingleQuoteString() {
         SQLException exception = expectThrows(SQLException.class, () -> SqlQueryParameterAnalyzer.parametersCount("SELECT ' '' '' "));
         assertEquals("Cannot parse given sql; unclosed string", exception.getMessage());
     }

x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/TypeConverterTests.java

@@ -11,17 +11,19 @@ import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
 import org.elasticsearch.test.ESTestCase;
 
+import java.sql.Date;
 import java.sql.Timestamp;
-import java.time.Clock;
-import java.time.Duration;
 import java.time.ZoneId;
 import java.time.ZonedDateTime;
 
+import static org.elasticsearch.xpack.sql.jdbc.JdbcTestUtils.nowWithMillisResolution;
 import static org.hamcrest.Matchers.instanceOf;
 
 
 public class TypeConverterTests extends ESTestCase {
 
+    private static final ZoneId UTC = ZoneId.of("Z");
+
     public void testFloatAsNative() throws Exception {
         assertThat(convertAsNative(42.0f, EsType.FLOAT), instanceOf(Float.class));
         assertThat(convertAsNative(42.0, EsType.FLOAT), instanceOf(Float.class));
@@ -41,9 +43,22 @@ public class TypeConverterTests extends ESTestCase {
     }
 
     public void testTimestampAsNative() throws Exception {
-        ZonedDateTime now = ZonedDateTime.now(Clock.tick(Clock.system(ZoneId.of("Z")), Duration.ofMillis(1)));
+        ZonedDateTime now = nowWithMillisResolution(UTC);
-        assertThat(convertAsNative(now, EsType.DATETIME), instanceOf(Timestamp.class));
+        Object nativeObject = convertAsNative(now, EsType.DATETIME);
-        assertEquals(now.toInstant().toEpochMilli(), ((Timestamp) convertAsNative(now, EsType.DATETIME)).getTime());
+        assertThat(nativeObject, instanceOf(Timestamp.class));
+        assertEquals(now.toInstant().toEpochMilli(), ((Timestamp) nativeObject).getTime());
+    }
+
+    public void testDateAsNative() throws Exception {
+        ZonedDateTime now = nowWithMillisResolution(UTC);
+        Object nativeObject = convertAsNative(now, EsType.DATE);
+        assertThat(nativeObject, instanceOf(Date.class));
+        assertEquals(now.toLocalDate().atStartOfDay(UTC).toInstant().toEpochMilli(), ((Date) nativeObject).getTime());
+
+        now = nowWithMillisResolution(ZoneId.of("Etc/GMT-10"));
+        nativeObject = convertAsNative(now, EsType.DATE);
+        assertThat(nativeObject, instanceOf(Date.class));
+        assertEquals(now.toLocalDate().atStartOfDay(ZoneId.of("Etc/GMT-10")).toInstant().toEpochMilli(), ((Date) nativeObject).getTime());
     }
 
     private Object convertAsNative(Object value, EsType type) throws Exception {

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

@@ -93,6 +93,12 @@ public abstract class JdbcIntegrationTestCase extends ESRestTestCase {
         client().performRequest(request);
     }
 
+    public static void delete(String index, String documentId) throws IOException {
+        Request request = new Request("DELETE", "/" + index + "/_doc/" + documentId);
+        request.addParameter("refresh", "true");
+        client().performRequest(request);
+    }
+
     protected String clusterName() {
         try {
             String response = EntityUtils.toString(client().performRequest(new Request("GET", "/")).getEntity());

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

@@ -1006,7 +1006,67 @@ public class ResultSetTestCase extends JdbcIntegrationTestCase {
             assertNull(results.getTimestamp("test_date"));
         });
     }
-    
+
+    public void testScalarOnDates() throws Exception {
+        createIndex("test");
+        updateMapping("test", builder -> builder.startObject("test_date").field("type", "date").endObject());
+
+        // 2018-03-12 17:00:00 UTC
+        Long dateInMillis = 1520874000000L;
+        index("test", "1", builder -> builder.field("test_date", dateInMillis));
+
+        // UTC +10 hours
+        String timeZoneId1 = "Etc/GMT-10";
+        Calendar connCalendar1 = Calendar.getInstance(TimeZone.getTimeZone(timeZoneId1), Locale.ROOT);
+
+        doWithQueryAndTimezone("SELECT test_date, DAY_OF_MONTH(test_date) as day FROM test", timeZoneId1, results -> {
+            results.next();
+            connCalendar1.setTimeInMillis(dateInMillis);
+            connCalendar1.set(HOUR_OF_DAY, 0);
+            connCalendar1.set(MINUTE, 0);
+            connCalendar1.set(SECOND, 0);
+            connCalendar1.set(MILLISECOND, 0);
+
+            assertEquals(new java.sql.Date(connCalendar1.getTimeInMillis()), results.getDate("test_date"));
+            assertEquals(new java.sql.Date(connCalendar1.getTimeInMillis()), results.getDate(1));
+            assertEquals(new java.sql.Date(dateInMillis - (dateInMillis % 86400000L)), results.getObject("test_date", java.sql.Date.class));
+            assertEquals(new java.sql.Date(dateInMillis - (dateInMillis % 86400000L)), results.getObject(1, java.sql.Date.class));
+
+            // +1 day
+            assertEquals(13, results.getInt("day"));
+        });
+
+        delete("test", "1");
+
+        // 2018-03-12 05:00:00 UTC
+        Long dateInMillis2 = 1520830800000L;
+        index("test", "1", builder -> builder.field("test_date", dateInMillis2));
+
+        // UTC -10 hours
+        String timeZoneId2 = "Etc/GMT+10";
+        Calendar connCalendar2 = Calendar.getInstance(TimeZone.getTimeZone(timeZoneId2), Locale.ROOT);
+
+
+        doWithQueryAndTimezone("SELECT test_date, DAY_OF_MONTH(test_date) as day FROM test", timeZoneId2, results -> {
+            results.next();
+            connCalendar2.setTimeInMillis(dateInMillis2);
+            connCalendar2.set(HOUR_OF_DAY, 0);
+            connCalendar2.set(MINUTE, 0);
+            connCalendar2.set(SECOND, 0);
+            connCalendar2.set(MILLISECOND, 0);
+
+            assertEquals(new java.sql.Date(connCalendar2.getTimeInMillis()), results.getDate("test_date"));
+            assertEquals(new java.sql.Date(connCalendar2.getTimeInMillis()), results.getDate(1));
+            assertEquals(new java.sql.Date(dateInMillis2 - (dateInMillis2 % 86400000L)),
+                results.getObject("test_date", java.sql.Date.class));
+            assertEquals(new java.sql.Date(dateInMillis2 - (dateInMillis2 % 86400000L)),
+                results.getObject(1, java.sql.Date.class));
+
+            // -1 day
+            assertEquals(11, results.getInt("day"));
+        });
+    }
+
     public void testValidGetObjectCalls() throws Exception {
         createIndex("test");
         updateMappingForNumericValuesTests("test");

x-pack/plugin/sql/qa/src/main/resources/date.csv-spec

@@ -3,7 +3,7 @@
 //
 
 currentDateKeywordWithDivision
-SELECT YEAR(CURRENT_TIMESTAMP) / 1000 AS result;
+SELECT YEAR(CURRENT_DATE) / 1000 AS result;
 
     result
 ---------------
@@ -11,7 +11,7 @@ SELECT YEAR(CURRENT_TIMESTAMP) / 1000 AS result;
 ;
 
 currentDateFunctionNoArgsWithDivision
-SELECT YEAR(CURRENT_TIMESTAMP()) / 1000 AS result;
+SELECT YEAR(CURRENT_DATE()) / 1000 AS result;
 
     result
 ---------------

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataType.java

@@ -43,7 +43,7 @@ public enum DataType {
     OBJECT(        "object",         JDBCType.STRUCT,    -1,                0,                 0,  false, false, false),
     NESTED(        "nested",         JDBCType.STRUCT,    -1,                0,                 0,  false, false, false),
     BINARY(        "binary",         JDBCType.VARBINARY, -1,                Integer.MAX_VALUE, 0,  false, false, false),
-    DATE(                            JDBCType.DATE,      Long.BYTES,        10,                10, false, false, true),
+    DATE(                            JDBCType.DATE,      Long.BYTES,        24,                24, false, false, true),
     // since ODBC and JDBC interpret precision for Date as display size
     // the precision is 23 (number of chars in ISO8601 with millis) + Z (the UTC timezone)
     // see https://github.com/elastic/elasticsearch/issues/30386#issuecomment-386807288