Skip to content

Commit c1e81e9

Browse files
astefanastefan
authored
SQL: adds format parameter to range queries for constant date comparisons (#45326)
* Add format parameter to the range queries built for CURRENT_* functions used in comparison conditions * Use range queries for date fields equality/non-equality as well.
1 parent f2ab41f commit c1e81e9

File tree

11 files changed

+308
-47
lines changed

11 files changed

+308
-47
lines changed

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
package org.elasticsearch.xpack.sql.qa.multi_node;
8+
9+
import org.elasticsearch.xpack.sql.qa.CustomDateFormatTestCase;
10+
11+
public class CustomDateFormatIT extends CustomDateFormatTestCase {
12+
13+
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@
2525
import java.util.Map;
2626

2727
import static java.util.Collections.singletonList;
28-
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.columnInfo;
29-
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.mode;
30-
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.randomMode;
28+
import static org.elasticsearch.xpack.sql.qa.rest.BaseRestSqlTestCase.mode;
29+
import static org.elasticsearch.xpack.sql.qa.rest.BaseRestSqlTestCase.randomMode;
3130
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.SQL_QUERY_REST_ENDPOINT;
31+
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.columnInfo;
3232

3333
/**
3434
* Tests specific to multiple nodes.

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@
2929
import java.util.Map;
3030
import java.util.stream.Collectors;
3131

32+
import static org.elasticsearch.xpack.sql.qa.rest.BaseRestSqlTestCase.mode;
33+
import static org.elasticsearch.xpack.sql.qa.rest.BaseRestSqlTestCase.randomMode;
3234
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.SQL_QUERY_REST_ENDPOINT;
3335
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.columnInfo;
34-
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.mode;
35-
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.randomMode;
3636
import static org.hamcrest.Matchers.containsString;
3737
import static org.hamcrest.Matchers.empty;
3838
import static org.hamcrest.Matchers.equalTo;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,10 @@
3434
import java.util.List;
3535
import java.util.Map;
3636

37+
import static org.elasticsearch.xpack.sql.qa.rest.BaseRestSqlTestCase.mode;
38+
import static org.elasticsearch.xpack.sql.qa.rest.BaseRestSqlTestCase.randomMode;
3739
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.SQL_QUERY_REST_ENDPOINT;
3840
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.columnInfo;
39-
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.mode;
40-
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.randomMode;
4141

4242
public class UserFunctionIT extends ESRestTestCase {
4343

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
package org.elasticsearch.xpack.sql.qa.single_node;
8+
9+
import org.elasticsearch.xpack.sql.qa.CustomDateFormatTestCase;
10+
11+
public class CustomDateFormatIT extends CustomDateFormatTestCase {
12+
13+
}

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

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
package org.elasticsearch.xpack.sql.qa;
8+
9+
import org.apache.http.entity.ContentType;
10+
import org.apache.http.entity.StringEntity;
11+
import org.elasticsearch.client.Request;
12+
import org.elasticsearch.client.Response;
13+
import org.elasticsearch.common.Strings;
14+
import org.elasticsearch.common.bytes.BytesArray;
15+
import org.elasticsearch.common.time.DateUtils;
16+
import org.elasticsearch.common.xcontent.XContentBuilder;
17+
import org.elasticsearch.common.xcontent.json.JsonXContent;
18+
import org.elasticsearch.xpack.sql.qa.jdbc.JdbcIntegrationTestCase;
19+
import org.elasticsearch.xpack.sql.qa.rest.BaseRestSqlTestCase;
20+
import org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase;
21+
22+
import java.io.IOException;
23+
import java.io.InputStream;
24+
import java.time.format.DateTimeFormatter;
25+
import java.util.Locale;
26+
27+
/*
28+
* Test class that covers the NOW()/CURRENT_DATE()/CURRENT_TIME() family of functions in a comparison condition
29+
* with different timezones and custom date formats for the date fields in ES.
30+
*/
31+
public abstract class CustomDateFormatTestCase extends BaseRestSqlTestCase {
32+
33+
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",
34+
"yyyy-MM-dd'T'HH:mm:ss'T'VV'T'z"};
35+
private static String[] nowFunctions = new String[] {"NOW()", "CURRENT_DATE()", "CURRENT_TIME()", "CURRENT_TIMESTAMP()"};
36+
private static String[] operators = new String[] {" < ", " > ", " <= ", " >= ", " = ", " != "};
37+
38+
public void testCustomDateFormatsWithNowFunctions() throws IOException {
39+
createIndex();
40+
String[] docs = new String[customFormats.length];
41+
String zID = JdbcIntegrationTestCase.randomKnownTimeZone();
42+
StringBuilder datesConditions = new StringBuilder();
43+
44+
for (int i = 0; i < customFormats.length; i++) {
45+
String field = "date_" + i;
46+
docs[i] = "{\"" + field + "\":\"" +
47+
DateTimeFormatter.ofPattern(customFormats[i], Locale.ROOT).format(DateUtils.nowWithMillisResolution()) + "\"}";
48+
datesConditions.append(i > 0 ? " OR " : "").append(field + randomFrom(operators) + randomFrom(nowFunctions));
49+
}
50+
51+
index(docs);
52+
53+
Request request = new Request("POST", RestSqlTestCase.SQL_QUERY_REST_ENDPOINT);
54+
request.setEntity(new StringEntity("{\"query\":\"SELECT COUNT(*) AS c FROM test WHERE "
55+
+ datesConditions.toString() + "\""
56+
+ mode("plain")
57+
+ ",\"time_zone\":\"" + zID + "\"" + "}", ContentType.APPLICATION_JSON));
58+
59+
Response response = client().performRequest(request);
60+
String expectedJsonSnippet = "{\"columns\":[{\"name\":\"c\",\"type\":\"long\"}],\"rows\":[[";
61+
try (InputStream content = response.getEntity().getContent()) {
62+
String actualJson = new BytesArray(content.readAllBytes()).utf8ToString();
63+
// we just need to get a response that's not a date parsing error
64+
assertTrue(actualJson.startsWith(expectedJsonSnippet));
65+
}
66+
}
67+
68+
private void createIndex() throws IOException {
69+
Request request = new Request("PUT", "/test");
70+
XContentBuilder index = JsonXContent.contentBuilder().prettyPrint().startObject();
71+
72+
index.startObject("mappings"); {
73+
index.startObject("properties"); {
74+
for (int i = 0; i < customFormats.length; i++) {
75+
String fieldName = "date_" + i;
76+
index.startObject(fieldName); {
77+
index.field("type", "date");
78+
index.field("format", customFormats[i]);
79+
}
80+
index.endObject();
81+
}
82+
index.endObject();
83+
}
84+
}
85+
index.endObject();
86+
index.endObject();
87+
88+
request.setJsonEntity(Strings.toString(index));
89+
client().performRequest(request);
90+
}
91+
}

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

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
import org.elasticsearch.common.xcontent.XContentBuilder;
1515
import org.elasticsearch.common.xcontent.XContentHelper;
1616
import org.elasticsearch.common.xcontent.json.JsonXContent;
17-
import org.elasticsearch.test.rest.ESRestTestCase;
17+
import org.elasticsearch.xpack.sql.qa.rest.BaseRestSqlTestCase;
1818
import org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase;
1919

2020
import java.io.IOException;
@@ -37,7 +37,7 @@
3737
* and which can affect the outcome of _source extraction and parsing when retrieving
3838
* values from Elasticsearch.
3939
*/
40-
public abstract class FieldExtractorTestCase extends ESRestTestCase {
40+
public abstract class FieldExtractorTestCase extends BaseRestSqlTestCase {
4141

4242
/*
4343
* "text_field": {
@@ -800,18 +800,6 @@ private void createIndexWithFieldTypeAndProperties(String type, Map<String, Map<
800800
client().performRequest(request);
801801
}
802802

803-
private void index(String... docs) throws IOException {
804-
Request request = new Request("POST", "/test/_bulk");
805-
request.addParameter("refresh", "true");
806-
StringBuilder bulk = new StringBuilder();
807-
for (String doc : docs) {
808-
bulk.append("{\"index\":{}\n");
809-
bulk.append(doc + "\n");
810-
}
811-
request.setJsonEntity(bulk.toString());
812-
client().performRequest(request);
813-
}
814-
815803
private Request buildRequest(String query) {
816804
Request request = new Request("POST", RestSqlTestCase.SQL_QUERY_REST_ENDPOINT);
817805
request.addParameter("error_trace", "true");

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
package org.elasticsearch.xpack.sql.qa.rest;
8+
9+
import org.elasticsearch.client.Request;
10+
import org.elasticsearch.common.Strings;
11+
import org.elasticsearch.test.rest.ESRestTestCase;
12+
import org.elasticsearch.xpack.sql.proto.StringUtils;
13+
14+
import java.io.IOException;
15+
16+
public abstract class BaseRestSqlTestCase extends ESRestTestCase {
17+
18+
protected void index(String... docs) throws IOException {
19+
Request request = new Request("POST", "/test/_bulk");
20+
request.addParameter("refresh", "true");
21+
StringBuilder bulk = new StringBuilder();
22+
for (String doc : docs) {
23+
bulk.append("{\"index\":{}\n");
24+
bulk.append(doc + "\n");
25+
}
26+
request.setJsonEntity(bulk.toString());
27+
client().performRequest(request);
28+
}
29+
30+
public static String mode(String mode) {
31+
return Strings.isEmpty(mode) ? StringUtils.EMPTY : ",\"mode\":\"" + mode + "\"";
32+
}
33+
34+
public static String randomMode() {
35+
return randomFrom(StringUtils.EMPTY, "jdbc", "plain");
36+
}
37+
}

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

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,12 @@
1414
import org.elasticsearch.client.Response;
1515
import org.elasticsearch.client.ResponseException;
1616
import org.elasticsearch.common.CheckedSupplier;
17-
import org.elasticsearch.common.Strings;
1817
import org.elasticsearch.common.bytes.BytesArray;
1918
import org.elasticsearch.common.collect.Tuple;
2019
import org.elasticsearch.common.io.Streams;
2120
import org.elasticsearch.common.xcontent.XContentHelper;
2221
import org.elasticsearch.common.xcontent.json.JsonXContent;
2322
import org.elasticsearch.test.NotEqualMessageBuilder;
24-
import org.elasticsearch.test.rest.ESRestTestCase;
2523
import org.elasticsearch.xpack.sql.proto.StringUtils;
2624
import org.elasticsearch.xpack.sql.qa.ErrorsTestCase;
2725
import org.hamcrest.Matcher;
@@ -49,7 +47,7 @@
4947
* Integration test for the rest sql action. The one that speaks json directly to a
5048
* user rather than to the JDBC driver or CLI.
5149
*/
52-
public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTestCase {
50+
public abstract class RestSqlTestCase extends BaseRestSqlTestCase implements ErrorsTestCase {
5351

5452
public static String SQL_QUERY_REST_ENDPOINT = org.elasticsearch.xpack.sql.proto.Protocol.SQL_QUERY_REST_ENDPOINT;
5553
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 {
892890
return XContentHelper.convertToMap(JsonXContent.jsonXContent, content, false);
893891
}
894892
}
895-
896-
public static String randomMode() {
897-
return randomFrom(StringUtils.EMPTY, "jdbc", "plain");
898-
}
899-
900-
public static String mode(String mode) {
901-
return Strings.isEmpty(mode) ? StringUtils.EMPTY : ",\"mode\":\"" + mode + "\"";
902-
}
903-
904-
protected void index(String... docs) throws IOException {
905-
Request request = new Request("POST", "/test/_bulk");
906-
request.addParameter("refresh", "true");
907-
StringBuilder bulk = new StringBuilder();
908-
for (String doc : docs) {
909-
bulk.append("{\"index\":{}\n");
910-
bulk.append(doc + "\n");
911-
}
912-
request.setJsonEntity(bulk.toString());
913-
client().performRequest(request);
914-
}
915893
}

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

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66
package org.elasticsearch.xpack.sql.planner;
77

8+
import org.elasticsearch.common.time.DateFormatter;
89
import org.elasticsearch.geometry.Geometry;
910
import org.elasticsearch.geometry.Point;
1011
import org.elasticsearch.search.sort.SortOrder;
@@ -107,6 +108,8 @@
107108
import org.elasticsearch.xpack.sql.util.DateUtils;
108109
import org.elasticsearch.xpack.sql.util.ReflectionUtils;
109110

111+
import java.time.OffsetTime;
112+
import java.time.ZonedDateTime;
110113
import java.util.Arrays;
111114
import java.util.LinkedHashMap;
112115
import java.util.List;
@@ -121,6 +124,9 @@
121124

122125
final class QueryTranslator {
123126

127+
public static final String DATE_FORMAT = "strict_date_time";
128+
public static final String TIME_FORMAT = "strict_hour_minute_second_millis";
129+
124130
private QueryTranslator(){}
125131

126132
private static final List<ExpressionTranslator<?>> QUERY_TRANSLATORS = Arrays.asList(
@@ -660,6 +666,25 @@ private static Query translateQuery(BinaryComparison bc) {
660666
String name = nameOf(bc.left());
661667
Object value = valueOf(bc.right());
662668
String format = dateFormat(bc.left());
669+
boolean isDateLiteralComparison = false;
670+
671+
// for a date constant comparison, we need to use a format for the date, to make sure that the format is the same
672+
// no matter the timezone provided by the user
673+
if ((value instanceof ZonedDateTime || value instanceof OffsetTime) && format == null) {
674+
DateFormatter formatter;
675+
if (value instanceof ZonedDateTime) {
676+
formatter = DateFormatter.forPattern(DATE_FORMAT);
677+
// RangeQueryBuilder accepts an Object as its parameter, but it will call .toString() on the ZonedDateTime instance
678+
// which can have a slightly different format depending on the ZoneId used to create the ZonedDateTime
679+
// Since RangeQueryBuilder can handle date as String as well, we'll format it as String and provide the format as well.
680+
value = formatter.format((ZonedDateTime) value);
681+
} else {
682+
formatter = DateFormatter.forPattern(TIME_FORMAT);
683+
value = formatter.format((OffsetTime) value);
684+
}
685+
format = formatter.pattern();
686+
isDateLiteralComparison = true;
687+
}
663688

664689
// Possible geo optimization
665690
if (bc.left() instanceof StDistance && value instanceof Number) {
@@ -697,10 +722,16 @@ private static Query translateQuery(BinaryComparison bc) {
697722
// (which is important for strings)
698723
name = ((FieldAttribute) bc.left()).exactAttribute().name();
699724
}
700-
Query query = new TermQuery(source, name, value);
725+
Query query;
726+
if (isDateLiteralComparison == true) {
727+
// dates equality uses a range query because it's the one that has a "format" parameter
728+
query = new RangeQuery(source, name, value, true, value, true, format);
729+
} else {
730+
query = new TermQuery(source, name, value);
731+
}
701732
if (bc instanceof NotEquals) {
702733
query = new NotQuery(source, query);
703-
}
734+
}
704735
return query;
705736
}
706737

0 commit comments

Comments
 (0)