SQL: adds format parameter to range queries for constant date comparisons

x-pack/plugin/sql/qa/multi-node/src/test/java/org/elasticsearch/xpack/sql/qa/multi_node/CustomDateFormatIT.java

@@ -0,0 +1,13 @@
+/*
+ * 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.qa.multi_node;
+
+import org.elasticsearch.xpack.sql.qa.CustomDateFormatTestCase;
+
+public class CustomDateFormatIT extends CustomDateFormatTestCase {
+
+}

x-pack/plugin/sql/qa/multi-node/src/test/java/org/elasticsearch/xpack/sql/qa/multi_node/RestSqlMultinodeIT.java

@@ -25,10 +25,10 @@
 import java.util.Map;
 
 import static java.util.Collections.singletonList;
-import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.columnInfo;
-import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.mode;
-import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.randomMode;
+import static org.elasticsearch.xpack.sql.qa.rest.BaseRestSqlTestCase.mode;
+import static org.elasticsearch.xpack.sql.qa.rest.BaseRestSqlTestCase.randomMode;
 import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.SQL_QUERY_REST_ENDPOINT;
+import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.columnInfo;
 
 /**
  * Tests specific to multiple nodes.

x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/RestSqlSecurityIT.java

@@ -29,10 +29,10 @@
 import java.util.Map;
 import java.util.stream.Collectors;
 
+import static org.elasticsearch.xpack.sql.qa.rest.BaseRestSqlTestCase.mode;
+import static org.elasticsearch.xpack.sql.qa.rest.BaseRestSqlTestCase.randomMode;
 import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.SQL_QUERY_REST_ENDPOINT;
 import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.columnInfo;
-import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.mode;
-import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.randomMode;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.equalTo;

x-pack/plugin/sql/qa/security/src/test/java/org/elasticsearch/xpack/sql/qa/security/UserFunctionIT.java

@@ -34,10 +34,10 @@
 import java.util.List;
 import java.util.Map;
 
+import static org.elasticsearch.xpack.sql.qa.rest.BaseRestSqlTestCase.mode;
+import static org.elasticsearch.xpack.sql.qa.rest.BaseRestSqlTestCase.randomMode;
 import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.SQL_QUERY_REST_ENDPOINT;
 import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.columnInfo;
-import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.mode;
-import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.randomMode;
 
 public class UserFunctionIT extends ESRestTestCase {
 

x-pack/plugin/sql/qa/single-node/src/test/java/org/elasticsearch/xpack/sql/qa/single_node/CustomDateFormatIT.java

@@ -0,0 +1,13 @@
+/*
+ * 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.qa.single_node;
+
+import org.elasticsearch.xpack.sql.qa.CustomDateFormatTestCase;
+
+public class CustomDateFormatIT extends CustomDateFormatTestCase {
+
+}

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

@@ -0,0 +1,91 @@
+/*
+ * 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.qa;
+
+import org.apache.http.entity.ContentType;
+import org.apache.http.entity.StringEntity;
+import org.elasticsearch.client.Request;
+import org.elasticsearch.client.Response;
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.bytes.BytesArray;
+import org.elasticsearch.common.time.DateUtils;
+import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.json.JsonXContent;
+import org.elasticsearch.xpack.sql.qa.jdbc.JdbcIntegrationTestCase;
+import org.elasticsearch.xpack.sql.qa.rest.BaseRestSqlTestCase;
+import org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.time.format.DateTimeFormatter;
+import java.util.Locale;
+
+/*
+ * Test class that covers the NOW()/CURRENT_DATE()/CURRENT_TIME() family of functions in a comparison condition
+ * with different timezones and custom date formats for the date fields in ES.
+ */
+public abstract class CustomDateFormatTestCase extends BaseRestSqlTestCase {
+
+    private static String[] customFormats = new String[] {"HH:mm yyyy-MM-dd", "HH:mm:ss yyyy-dd-MM", "HH:mm:ss VV", "HH:mm:ss VV z",
+            "yyyy-MM-dd'T'HH:mm:ss'T'VV'T'z"};
+    private static String[] nowFunctions = new String[] {"NOW()", "CURRENT_DATE()", "CURRENT_TIME()", "CURRENT_TIMESTAMP()"};
+    private static String[] operators = new String[] {" < ", " > ", " <= ", " >= ", " = ", " != "};
+
+    public void testCustomDateFormatsWithNowFunctions() throws IOException {
+        createIndex();
+        String[] docs = new String[customFormats.length];
+        String zID = JdbcIntegrationTestCase.randomKnownTimeZone();
+        StringBuilder datesConditions = new StringBuilder();
+
+        for (int i = 0; i < customFormats.length; i++) {
+            String field = "date_" + i;
+            docs[i] = "{\"" + field + "\":\"" +
+                    DateTimeFormatter.ofPattern(customFormats[i], Locale.ROOT).format(DateUtils.nowWithMillisResolution()) + "\"}";
+            datesConditions.append(i > 0 ? " OR " : "").append(field + randomFrom(operators) + randomFrom(nowFunctions));
+        }
+
+        index(docs);
+
+        Request request = new Request("POST", RestSqlTestCase.SQL_QUERY_REST_ENDPOINT);
+        request.setEntity(new StringEntity("{\"query\":\"SELECT COUNT(*) AS c FROM test WHERE "
+                + datesConditions.toString() + "\""
+                + mode("plain")
+                + ",\"time_zone\":\"" + zID + "\"" + "}", ContentType.APPLICATION_JSON));
+
+        Response response = client().performRequest(request);
+        String expectedJsonSnippet = "{\"columns\":[{\"name\":\"c\",\"type\":\"long\"}],\"rows\":[[";
+        try (InputStream content = response.getEntity().getContent()) {
+            String actualJson = new BytesArray(content.readAllBytes()).utf8ToString();
+            // we just need to get a response that's not a date parsing error
+            assertTrue(actualJson.startsWith(expectedJsonSnippet));
+        }
+    }
+
+    private void createIndex() throws IOException {
+        Request request = new Request("PUT", "/test");      
+        XContentBuilder index = JsonXContent.contentBuilder().prettyPrint().startObject();
+
+        index.startObject("mappings"); {
+            index.startObject("properties"); {
+                for (int i = 0; i < customFormats.length; i++) {
+                    String fieldName = "date_" + i;
+                    index.startObject(fieldName); {
+                        index.field("type", "date");
+                        index.field("format", customFormats[i]);
+                    }
+                    index.endObject();
+                }
+            index.endObject();
+            }
+        }
+        index.endObject();
+        index.endObject();
+
+        request.setJsonEntity(Strings.toString(index));
+        client().performRequest(request);
+    }
+}

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

@@ -14,7 +14,7 @@
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
-import org.elasticsearch.test.rest.ESRestTestCase;
+import org.elasticsearch.xpack.sql.qa.rest.BaseRestSqlTestCase;
 import org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase;
 
 import java.io.IOException;
@@ -37,7 +37,7 @@
  * and which can affect the outcome of _source extraction and parsing when retrieving
  * values from Elasticsearch.
  */
-public abstract class FieldExtractorTestCase extends ESRestTestCase {
+public abstract class FieldExtractorTestCase extends BaseRestSqlTestCase {
 
     /*
      *    "text_field": {
@@ -800,18 +800,6 @@ private void createIndexWithFieldTypeAndProperties(String type, Map<String, Map<
         client().performRequest(request);
     }
 
-    private void index(String... docs) throws IOException {
-        Request request = new Request("POST", "/test/_bulk");
-        request.addParameter("refresh", "true");
-        StringBuilder bulk = new StringBuilder();
-        for (String doc : docs) {
-            bulk.append("{\"index\":{}\n");
-            bulk.append(doc + "\n");
-        }
-        request.setJsonEntity(bulk.toString());
-        client().performRequest(request);
-    }
-
     private Request buildRequest(String query) {
         Request request = new Request("POST", RestSqlTestCase.SQL_QUERY_REST_ENDPOINT);
         request.addParameter("error_trace", "true");

x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/BaseRestSqlTestCase.java

@@ -0,0 +1,37 @@
+/*
+ * 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.qa.rest;
+
+import org.elasticsearch.client.Request;
+import org.elasticsearch.common.Strings;
+import org.elasticsearch.test.rest.ESRestTestCase;
+import org.elasticsearch.xpack.sql.proto.StringUtils;
+
+import java.io.IOException;
+
+public abstract class BaseRestSqlTestCase extends ESRestTestCase {
+
+    protected void index(String... docs) throws IOException {
+        Request request = new Request("POST", "/test/_bulk");
+        request.addParameter("refresh", "true");
+        StringBuilder bulk = new StringBuilder();
+        for (String doc : docs) {
+            bulk.append("{\"index\":{}\n");
+            bulk.append(doc + "\n");
+        }
+        request.setJsonEntity(bulk.toString());
+        client().performRequest(request);
+    }
+
+    public static String mode(String mode) {
+        return Strings.isEmpty(mode) ? StringUtils.EMPTY : ",\"mode\":\"" + mode + "\"";
+    }
+
+    public static String randomMode() {
+        return randomFrom(StringUtils.EMPTY, "jdbc", "plain");
+    }
+}

x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java

@@ -14,14 +14,12 @@
 import org.elasticsearch.client.Response;
 import org.elasticsearch.client.ResponseException;
 import org.elasticsearch.common.CheckedSupplier;
-import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.io.Streams;
 import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
 import org.elasticsearch.test.NotEqualMessageBuilder;
-import org.elasticsearch.test.rest.ESRestTestCase;
 import org.elasticsearch.xpack.sql.proto.StringUtils;
 import org.elasticsearch.xpack.sql.qa.ErrorsTestCase;
 import org.hamcrest.Matcher;
@@ -49,7 +47,7 @@
  * Integration test for the rest sql action. The one that speaks json directly to a
  * user rather than to the JDBC driver or CLI.
  */
-public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTestCase {
+public abstract class RestSqlTestCase extends BaseRestSqlTestCase implements ErrorsTestCase {
 
     public static String SQL_QUERY_REST_ENDPOINT = org.elasticsearch.xpack.sql.proto.Protocol.SQL_QUERY_REST_ENDPOINT;
     private static String SQL_TRANSLATE_REST_ENDPOINT = org.elasticsearch.xpack.sql.proto.Protocol.SQL_TRANSLATE_REST_ENDPOINT;
@@ -892,24 +890,4 @@ private static Map<String, Object> searchStats() throws IOException {
             return XContentHelper.convertToMap(JsonXContent.jsonXContent, content, false);
         }
     }
-
-    public static String randomMode() {
-        return randomFrom(StringUtils.EMPTY, "jdbc", "plain");
-    }
-
-    public static String mode(String mode) {
-        return Strings.isEmpty(mode) ? StringUtils.EMPTY : ",\"mode\":\"" + mode + "\"";
-    }
-
-    protected void index(String... docs) throws IOException {
-        Request request = new Request("POST", "/test/_bulk");
-        request.addParameter("refresh", "true");
-        StringBuilder bulk = new StringBuilder();
-        for (String doc : docs) {
-            bulk.append("{\"index\":{}\n");
-            bulk.append(doc + "\n");
-        }
-        request.setJsonEntity(bulk.toString());
-        client().performRequest(request);
-    }
 }

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java

@@ -5,6 +5,7 @@
  */
 package org.elasticsearch.xpack.sql.planner;
 
+import org.elasticsearch.common.time.DateFormatter;
 import org.elasticsearch.geometry.Geometry;
 import org.elasticsearch.geometry.Point;
 import org.elasticsearch.search.sort.SortOrder;
@@ -107,6 +108,8 @@
 import org.elasticsearch.xpack.sql.util.DateUtils;
 import org.elasticsearch.xpack.sql.util.ReflectionUtils;
 
+import java.time.OffsetTime;
+import java.time.ZonedDateTime;
 import java.util.Arrays;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -121,6 +124,9 @@
 
 final class QueryTranslator {
 
+    public static final String DATE_FORMAT = "strict_date_time";
+    public static final String TIME_FORMAT = "strict_hour_minute_second_millis";
+
     private QueryTranslator(){}
 
     private static final List<ExpressionTranslator<?>> QUERY_TRANSLATORS = Arrays.asList(
@@ -660,6 +666,25 @@ private static Query translateQuery(BinaryComparison bc) {
             String name = nameOf(bc.left());
             Object value = valueOf(bc.right());
             String format = dateFormat(bc.left());
+            boolean isDateLiteralComparison = false;
+
+            // for a date constant comparison, we need to use a format for the date, to make sure that the format is the same
+            // no matter the timezone provided by the user
+            if ((value instanceof ZonedDateTime || value instanceof OffsetTime) && format == null) {
+                DateFormatter formatter;
+                if (value instanceof ZonedDateTime) {
+                    formatter = DateFormatter.forPattern(DATE_FORMAT);
+                    // RangeQueryBuilder accepts an Object as its parameter, but it will call .toString() on the ZonedDateTime instance
+                    // which can have a slightly different format depending on the ZoneId used to create the ZonedDateTime
+                    // Since RangeQueryBuilder can handle date as String as well, we'll format it as String and provide the format as well.
+                    value = formatter.format((ZonedDateTime) value);
+                } else {
+                    formatter = DateFormatter.forPattern(TIME_FORMAT); 
+                    value = formatter.format((OffsetTime) value);
+                }
+                format = formatter.pattern();
+                isDateLiteralComparison = true;
+            }
 
             // Possible geo optimization
             if (bc.left() instanceof StDistance && value instanceof Number) {
@@ -697,10 +722,16 @@ private static Query translateQuery(BinaryComparison bc) {
                     // (which is important for strings)
                     name = ((FieldAttribute) bc.left()).exactAttribute().name();
                 }
-                Query query = new TermQuery(source, name, value);
+                Query query;
+                if (isDateLiteralComparison == true) {
+                    // dates equality uses a range query because it's the one that has a "format" parameter
+                    query = new RangeQuery(source, name, value, true, value, true, format);
+                } else {
+                    query = new TermQuery(source, name, value);
+                }
                 if (bc instanceof NotEquals) {
                     query = new NotQuery(source, query);
-            }
+                }
                 return query;
             }