SQL: Fix serialization of JDBC prep statement date/time params (#56492)…

… (#56581)

The Date/Time related query params of a JDBC prepared statement
serialized using java.util.Date. The rules for serializing
`java.util.Date` objects though reside in
`XContentElasticsearchExtension` which is not available in the
jdbc jar as this class is in `server` module. Therefore, a
custom extension of the `XContentBuilderExtension` iface has been
added to the jdbc module/jar.

Moreover the sql's `qa` project had as dependency the `sql-action`
module which depends on `server` so the `XContentBuilderExtension`
was available for the integ tests hiding the real problem.

Previously, when a user was setting a `java.sql.Time` to the prepStmt,
the DataType used was `DATETIME` instead of `TIME` and therefore
prevented from filtering with a `TIME` casted field:
```
SELECT * FROM test WHERE date::TIME = ?
```

Fixes: #56084
(cherry picked from commit f8d8e97)

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

@@ -39,7 +39,10 @@
 import java.util.List;
 import java.util.Locale;
 
+import static java.time.ZoneOffset.UTC;
+
 class JdbcPreparedStatement extends JdbcStatement implements PreparedStatement {
+
     final PreparedQuery query;
 
     JdbcPreparedStatement(JdbcConnection con, JdbcConfiguration info, String sql) throws SQLException {
@@ -143,7 +146,7 @@ public void setDate(int parameterIndex, Date x) throws SQLException {
 
     @Override
     public void setTime(int parameterIndex, Time x) throws SQLException {
-        setObject(parameterIndex, x, Types.TIMESTAMP);
+        setObject(parameterIndex, x, Types.TIME);
     }
 
     @Override
@@ -251,15 +254,15 @@ public void setDate(int parameterIndex, Date x, Calendar cal) throws SQLExceptio
     @Override
     public void setTime(int parameterIndex, Time x, Calendar cal) throws SQLException {
         if (cal == null) {
-            setObject(parameterIndex, x, Types.TIMESTAMP);
+            setObject(parameterIndex, x, Types.TIME);
             return;
         }
         if (x == null) {
-            setNull(parameterIndex, Types.TIMESTAMP);
+            setNull(parameterIndex, Types.TIME);
             return;
         }
         // converting to UTC since this is what ES is storing internally
-        setObject(parameterIndex, new Time(TypeConverter.convertFromCalendarToUTC(x.getTime(), cal)), Types.TIMESTAMP);
+        setObject(parameterIndex, new Time(TypeConverter.convertFromCalendarToUTC(x.getTime(), cal)), Types.TIME);
     }
 
     @Override
@@ -366,7 +369,7 @@ private void setObject(int parameterIndex, Object x, EsType dataType, String typ
                 || x instanceof Time
                 || x instanceof java.util.Date)
         {
-            if (dataType == EsType.DATETIME) {
+            if (dataType == EsType.DATETIME || dataType == EsType.TIME) {
                 // converting to {@code java.util.Date} because this is the type supported by {@code XContentBuilder} for serialization
                 java.util.Date dateToSet;
                 if (x instanceof Timestamp) {
@@ -375,12 +378,9 @@ private void setObject(int parameterIndex, Object x, EsType dataType, String typ
                     dateToSet = ((Calendar) x).getTime();
                 } else if (x instanceof Date) {
                     dateToSet = new java.util.Date(((Date) x).getTime());
-                } else if (x instanceof LocalDateTime){
+                } else if (x instanceof LocalDateTime) {
                     LocalDateTime ldt = (LocalDateTime) x;
-                    Calendar cal = getDefaultCalendar();
-                    cal.set(ldt.getYear(), ldt.getMonthValue() - 1, ldt.getDayOfMonth(), ldt.getHour(), ldt.getMinute(), ldt.getSecond());
-
-                    dateToSet = cal.getTime();
+                    dateToSet = new java.util.Date(ldt.toInstant(UTC).toEpochMilli());
                 } else if (x instanceof Time) {
                     dateToSet = new java.util.Date(((Time) x).getTime());
                 } else {

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

@@ -60,7 +60,7 @@ private TypeUtils() {}
         aMap.put(GregorianCalendar.class, EsType.DATETIME);
         aMap.put(java.util.Date.class, EsType.DATETIME);
         aMap.put(java.sql.Date.class, EsType.DATETIME);
-        aMap.put(java.sql.Time.class, EsType.DATETIME);
+        aMap.put(java.sql.Time.class, EsType.TIME);
         aMap.put(LocalDateTime.class, EsType.DATETIME);
         CLASS_TO_TYPE = Collections.unmodifiableMap(aMap);
 

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

@@ -0,0 +1,42 @@
+/*
+ * 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 org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.XContentBuilderExtension;
+
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Function;
+
+/**
+ * Extension for SQL's JDBC specific classes that need to be
+ * encoded by {@link XContentBuilder} in a specific way.
+ */
+public class XContentSqlExtension implements XContentBuilderExtension {
+
+    @Override
+    public Map<Class<?>, XContentBuilder.Writer> getXContentWriters() {
+        Map<Class<?>, XContentBuilder.Writer> map = new HashMap<>();
+        map.put(Date.class, (b, v) -> b.value(((Date) v).getTime()));
+        return map;
+    }
+
+    @Override
+    public Map<Class<?>, XContentBuilder.HumanReadableTransformer> getXContentHumanReadableTransformers() {
+        return Collections.emptyMap();
+    }
+
+    @Override
+    public Map<Class<?>, Function<Object, Object>> getDateTransformers() {
+        Map<Class<?>, Function<Object, Object>> map = new HashMap<>();
+        map.put(Date.class, d -> ((Date) d).getTime());
+        return map;
+    }
+}

x-pack/plugin/sql/jdbc/src/main/resources/META-INF/services/org.elasticsearch.common.xcontent.XContentBuilderExtension

@@ -0,0 +1 @@
+org.elasticsearch.xpack.sql.jdbc.XContentSqlExtension

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

@@ -37,6 +37,7 @@
 import static org.elasticsearch.xpack.sql.jdbc.EsType.KEYWORD;
 import static org.elasticsearch.xpack.sql.jdbc.EsType.LONG;
 import static org.elasticsearch.xpack.sql.jdbc.EsType.SHORT;
+import static org.elasticsearch.xpack.sql.jdbc.EsType.TIME;
 
 public class JdbcPreparedStatementTests extends ESTestCase {
 
@@ -401,10 +402,15 @@ public void testSettingTimeValues() throws SQLException {
         JdbcPreparedStatement jps = createJdbcPreparedStatement();
 
         Time time = new Time(4675000);
+        jps.setTime(1, time);
+        assertEquals(time, value(jps));
+        assertEquals(TIME, jdbcType(jps));
+        assertTrue(value(jps) instanceof java.util.Date);
+
         Calendar nonDefaultCal = randomCalendar();
         jps.setTime(1, time, nonDefaultCal);
         assertEquals(4675000, convertFromUTCtoCalendar(((Date)value(jps)), nonDefaultCal));
-        assertEquals(DATETIME, jdbcType(jps));
+        assertEquals(TIME, jdbcType(jps));
         assertTrue(value(jps) instanceof java.util.Date);
 
         jps.setObject(1, time, Types.VARCHAR);

x-pack/plugin/sql/qa/build.gradle

@@ -9,7 +9,6 @@ dependencies {
   // JDBC testing dependencies
   compile project(path: xpackModule('sql:jdbc'))
 
-  compile project(path: xpackModule('sql:sql-action'))
   compile "net.sourceforge.csvjdbc:csvjdbc:${csvjdbcVersion}"
 
   // CLI testing dependencies

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

@@ -31,8 +31,10 @@
 import java.time.Instant;
 import java.time.LocalDate;
 import java.time.ZoneId;
+import java.time.ZoneOffset;
 import java.time.ZonedDateTime;
 import java.util.ArrayList;
+import java.util.Calendar;
 import java.util.EnumSet;
 import java.util.List;
 import java.util.jar.JarInputStream;
@@ -46,6 +48,7 @@ private JdbcTestUtils() {}
 
     private static final int MAX_WIDTH = 20;
 
+    static final ZoneId UTC = ZoneId.of("Z");
     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);
@@ -240,4 +243,18 @@ static Time asTime(long millis, ZoneId zoneId) {
         return new Time(ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), zoneId)
                 .toLocalTime().atDate(JdbcTestUtils.EPOCH).atZone(zoneId).toInstant().toEpochMilli());
     }
+
+    static long convertFromCalendarToUTC(long value, Calendar cal) {
+        if (cal == null) {
+            return value;
+        }
+        Calendar c = (Calendar) cal.clone();
+        c.setTimeInMillis(value);
+
+        ZonedDateTime convertedDateTime = ZonedDateTime
+            .ofInstant(c.toInstant(), c.getTimeZone().toZoneId())
+            .withZoneSameLocal(ZoneOffset.UTC);
+
+        return convertedDateTime.toInstant().toEpochMilli();
+    }
 }

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

@@ -6,26 +6,35 @@
 package org.elasticsearch.xpack.sql.qa.jdbc;
 
 import org.elasticsearch.common.collect.Tuple;
+import org.elasticsearch.common.settings.Settings;
 
+import java.io.IOException;
 import java.sql.Connection;
+import java.sql.Date;
 import java.sql.JDBCType;
 import java.sql.ParameterMetaData;
 import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
 import java.sql.SQLSyntaxErrorException;
+import java.sql.Time;
+import java.sql.Timestamp;
+import java.time.Instant;
+import java.time.LocalDateTime;
+import java.util.Calendar;
+import java.util.Locale;
+import java.util.StringJoiner;
 
+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.convertFromCalendarToUTC;
+import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.UTC;
 import static org.hamcrest.Matchers.startsWith;
 
 public class PreparedStatementTestCase extends JdbcIntegrationTestCase {
 
-    public void testSupportedTypes() throws Exception {
-        index("library", builder -> {
-            builder.field("name", "Don Quixote");
-            builder.field("page_count", 1072);
-        });
-
+    public void testSupportedTypes() throws SQLException {
         String stringVal = randomAlphaOfLength(randomIntBetween(0, 1000));
         int intVal = randomInt();
         long longVal = randomLong();
@@ -34,10 +43,23 @@ public void testSupportedTypes() throws Exception {
         boolean booleanVal = randomBoolean();
         byte byteVal = randomByte();
         short shortVal = randomShort();
+        long millis = randomNonNegativeLong();
+        Calendar calendarVal = Calendar.getInstance(randomTimeZone(), Locale.ROOT);
+        Timestamp timestampVal = new Timestamp(millis);
+        Timestamp timestampValWithCal = new Timestamp(convertFromCalendarToUTC(timestampVal.getTime(), calendarVal));
+        Date dateVal = asDate(millis, UTC);
+        Date dateValWithCal = asDate(convertFromCalendarToUTC(dateVal.getTime(), calendarVal), UTC);
+        Time timeVal = asTime(millis, UTC);
+        Time timeValWithCal = asTime(convertFromCalendarToUTC(timeVal.getTime(), calendarVal), UTC);
+        java.util.Date utilDateVal = new java.util.Date(millis);
+        LocalDateTime localDateTimeVal = LocalDateTime.ofInstant(Instant.ofEpochMilli(millis), UTC);
 
         try (Connection connection = esJdbc()) {
-            try (PreparedStatement statement = connection.prepareStatement(
-                    "SELECT ?, ?, ?, ?, ?, ?, ?, ?, ?, name FROM library WHERE page_count=?")) {
+            StringJoiner sql = new StringJoiner(",", "SELECT ", "");
+            for (int i = 0; i < 18; i++) {
+                sql.add("?");
+            }
+            try (PreparedStatement statement = connection.prepareStatement(sql.toString())) {
                 statement.setString(1, stringVal);
                 statement.setInt(2, intVal);
                 statement.setLong(3, longVal);
@@ -47,7 +69,15 @@ public void testSupportedTypes() throws Exception {
                 statement.setBoolean(7, booleanVal);
                 statement.setByte(8, byteVal);
                 statement.setShort(9, shortVal);
-                statement.setInt(10, 1072);
+                statement.setTimestamp(10, timestampVal);
+                statement.setTimestamp(11, timestampVal, calendarVal);
+                statement.setDate(12, dateVal);
+                statement.setDate(13, dateVal, calendarVal);
+                statement.setTime(14, timeVal);
+                statement.setTime(15, timeVal, calendarVal);
+                statement.setObject(16, calendarVal);
+                statement.setObject(17, utilDateVal);
+                statement.setObject(18, localDateTimeVal);
 
                 try (ResultSet results = statement.executeQuery()) {
                     ResultSetMetaData resultSetMetaData = results.getMetaData();
@@ -67,7 +97,70 @@ public void testSupportedTypes() throws Exception {
                     assertEquals(booleanVal, results.getBoolean(7));
                     assertEquals(byteVal, results.getByte(8));
                     assertEquals(shortVal, results.getShort(9));
-                    assertEquals("Don Quixote", results.getString(10));
+                    assertEquals(timestampVal, results.getTimestamp(10));
+                    assertEquals(timestampValWithCal, results.getTimestamp(11));
+                    assertEquals(dateVal, results.getDate(12));
+                    assertEquals(dateValWithCal, results.getDate(13));
+                    assertEquals(timeVal, results.getTime(14));
+                    assertEquals(timeValWithCal, results.getTime(15));
+                    assertEquals(new Timestamp(calendarVal.getTimeInMillis()), results.getObject(16));
+                    assertEquals(timestampVal, results.getObject(17));
+                    assertEquals(timestampVal, results.getObject(18));
+                    assertFalse(results.next());
+                }
+            }
+        }
+    }
+
+    public void testDatetime() throws IOException, SQLException {
+        long randomMillis = randomNonNegativeLong();
+        setupIndexForDateTimeTests(randomMillis);
+
+        try (Connection connection = esJdbc()) {
+            try (PreparedStatement statement = connection.prepareStatement("SELECT id, birth_date FROM emps WHERE birth_date = ?")) {
+                Object dateTimeParam = randomFrom(new Timestamp(randomMillis), new Date(randomMillis));
+                statement.setObject(1, dateTimeParam);
+                try (ResultSet results = statement.executeQuery()) {
+                    assertTrue(results.next());
+                    assertEquals(1002, results.getInt(1));
+                    assertEquals(new Timestamp(randomMillis), results.getTimestamp(2));
+                    assertFalse(results.next());
+                }
+            }
+        }
+    }
+
+    public void testDate() throws IOException, SQLException {
+        long randomMillis = randomNonNegativeLong();
+        setupIndexForDateTimeTests(randomMillis);
+
+        try (Connection connection = esJdbc()) {
+            try (PreparedStatement statement = connection.prepareStatement("SELECT id, birth_date FROM emps WHERE birth_date::date = ?")) {
+                statement.setDate(1, new Date(asDate(randomMillis, UTC).getTime()));
+                try (ResultSet results = statement.executeQuery()) {
+                    for (int i = 1; i <= 3; i++) {
+                        assertTrue(results.next());
+                        assertEquals(1000 + i, results.getInt(1));
+                        assertEquals(new Timestamp(testMillis(randomMillis, i)), results.getTimestamp(2));
+                    }
+                    assertFalse(results.next());
+                }
+            }
+        }
+    }
+
+    public void testTime() throws IOException, SQLException {
+        long randomMillis = randomNonNegativeLong();
+        setupIndexForDateTimeTests(randomMillis);
+
+        try (Connection connection = esJdbc()) {
+            try (PreparedStatement statement = connection.prepareStatement("SELECT id, birth_date FROM emps WHERE birth_date::time = ?")) {
+                Time time = JdbcTestUtils.asTime(randomMillis, UTC);
+                statement.setObject(1, time);
+                try (ResultSet results = statement.executeQuery()) {
+                    assertTrue(results.next());
+                    assertEquals(1002, results.getInt(1));
+                    assertEquals(new Timestamp(randomMillis), results.getTimestamp(2));
                     assertFalse(results.next());
                 }
             }
@@ -190,7 +283,7 @@ public void testSingleParameterMultipleTypes() throws Exception {
         }
     }
 
-    private Tuple<Integer, Object> execute(PreparedStatement statement) throws SQLException {
+    private Tuple<Integer, Object> execute(PreparedStatement statement) throws Exception {
         try (ResultSet results = statement.executeQuery()) {
             ResultSetMetaData resultSetMetaData = results.getMetaData();
             assertTrue(results.next());
@@ -199,4 +292,21 @@ private Tuple<Integer, Object> execute(PreparedStatement statement) throws SQLEx
             return result;
         }
     }
+
+    private static long testMillis(long randomMillis, int i) {
+        return randomMillis - 2 + i;
+    }
+
+    private static void setupIndexForDateTimeTests(long randomMillis) throws IOException {
+        String mapping = "\"properties\":{\"id\":{\"type\":\"integer\"},\"birth_date\":{\"type\":\"date\"}}";
+        createIndex("emps", Settings.EMPTY, mapping);
+        for (int i = 1; i <= 3; i++) {
+            int id = 1000 + i;
+            long testMillis = testMillis(randomMillis, i);
+            index("emps", "" + i, builder -> {
+                builder.field("id", id);
+                builder.field("birth_date", testMillis);
+            });
+        }
+    }
 }

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/DateUtils.java

@@ -69,7 +69,7 @@ public final class DateUtils {
             .optionalEnd()
             .toFormatter().withZone(UTC);
 
-    private static final DateFormatter UTC_DATE_TIME_FORMATTER = DateFormatter.forPattern("date_optional_time").withZone(UTC);
+    private static final DateFormatter UTC_DATE_TIME_FORMATTER = DateFormatter.forPattern("strict_date_optional_time").withZone(UTC);
     private static final int DEFAULT_PRECISION_FOR_CURRENT_FUNCTIONS = 3;
 
     private DateUtils() {}