Skip to content

Commit 9d12541

Browse files
astefanastefan
committed
Add "format" to "range" queries resulted from optimizing a logical AND (#48073)
(cherry picked from commit 020939a)
1 parent 76fdaf7 commit 9d12541

File tree

2 files changed

+78
-2
lines changed

2 files changed

+78
-2
lines changed

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

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,10 +106,12 @@
106106
import org.elasticsearch.xpack.sql.tree.Source;
107107
import org.elasticsearch.xpack.sql.util.Check;
108108
import org.elasticsearch.xpack.sql.util.DateUtils;
109+
import org.elasticsearch.xpack.sql.util.Holder;
109110
import org.elasticsearch.xpack.sql.util.ReflectionUtils;
110111

111112
import java.time.OffsetTime;
112113
import java.time.ZonedDateTime;
114+
import java.time.temporal.TemporalAccessor;
113115
import java.util.Arrays;
114116
import java.util.LinkedHashMap;
115117
import java.util.List;
@@ -800,9 +802,36 @@ protected QueryTranslation asQuery(Range r, boolean onAggs) {
800802
if (onAggs) {
801803
aggFilter = new AggFilter(at.id().toString(), r.asScript());
802804
} else {
805+
Holder<Object> lower = new Holder<>(valueOf(r.lower()));
806+
Holder<Object> upper = new Holder<>(valueOf(r.upper()));
807+
Holder<String> format = new Holder<>(dateFormat(r.value()));
808+
809+
// for a date constant comparison, we need to use a format for the date, to make sure that the format is the same
810+
// no matter the timezone provided by the user
811+
if (format.get() == null) {
812+
DateFormatter formatter = null;
813+
if (lower.get() instanceof ZonedDateTime || upper.get() instanceof ZonedDateTime) {
814+
formatter = DateFormatter.forPattern(DATE_FORMAT);
815+
} else if (lower.get() instanceof OffsetTime || upper.get() instanceof OffsetTime) {
816+
formatter = DateFormatter.forPattern(TIME_FORMAT);
817+
}
818+
if (formatter != null) {
819+
// RangeQueryBuilder accepts an Object as its parameter, but it will call .toString() on the ZonedDateTime
820+
// instance which can have a slightly different format depending on the ZoneId used to create the ZonedDateTime
821+
// Since RangeQueryBuilder can handle date as String as well, we'll format it as String and provide the format.
822+
if (lower.get() instanceof ZonedDateTime || lower.get() instanceof OffsetTime) {
823+
lower.set(formatter.format((TemporalAccessor) lower.get()));
824+
}
825+
if (upper.get() instanceof ZonedDateTime || upper.get() instanceof OffsetTime) {
826+
upper.set(formatter.format((TemporalAccessor) upper.get()));
827+
}
828+
format.set(formatter.pattern());
829+
}
830+
}
831+
803832
query = handleQuery(r, r.value(),
804-
() -> new RangeQuery(r.source(), nameOf(r.value()), valueOf(r.lower()), r.includeLower(),
805-
valueOf(r.upper()), r.includeUpper(), dateFormat(r.value())));
833+
() -> new RangeQuery(r.source(), nameOf(r.value()), lower.get(), r.includeLower(),
834+
upper.get(), r.includeUpper(), format.get()));
806835
}
807836
return new QueryTranslation(query, aggFilter);
808837
} else {

‎x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,22 +239,37 @@ public void testDateRangeCast() {
239239

240240
public void testDateRangeWithCurrentTimestamp() {
241241
testDateRangeWithCurrentFunctions("CURRENT_TIMESTAMP()", DATE_FORMAT, TestUtils.TEST_CFG.now());
242+
testDateRangeWithCurrentFunctions_AndRangeOptimization("CURRENT_TIMESTAMP()", DATE_FORMAT,
243+
TestUtils.TEST_CFG.now().minusDays(1L).minusSeconds(1L),
244+
TestUtils.TEST_CFG.now().plusDays(1L).plusSeconds(1L));
242245
}
243246

244247
public void testDateRangeWithCurrentDate() {
245248
testDateRangeWithCurrentFunctions("CURRENT_DATE()", DATE_FORMAT, DateUtils.asDateOnly(TestUtils.TEST_CFG.now()));
249+
testDateRangeWithCurrentFunctions_AndRangeOptimization("CURRENT_DATE()", DATE_FORMAT,
250+
DateUtils.asDateOnly(TestUtils.TEST_CFG.now().minusDays(2L)),
251+
DateUtils.asDateOnly(TestUtils.TEST_CFG.now().plusDays(1L)));
246252
}
247253

248254
public void testDateRangeWithToday() {
249255
testDateRangeWithCurrentFunctions("TODAY()", DATE_FORMAT, DateUtils.asDateOnly(TestUtils.TEST_CFG.now()));
256+
testDateRangeWithCurrentFunctions_AndRangeOptimization("TODAY()", DATE_FORMAT,
257+
DateUtils.asDateOnly(TestUtils.TEST_CFG.now().minusDays(2L)),
258+
DateUtils.asDateOnly(TestUtils.TEST_CFG.now().plusDays(1L)));
250259
}
251260

252261
public void testDateRangeWithNow() {
253262
testDateRangeWithCurrentFunctions("NOW()", DATE_FORMAT, TestUtils.TEST_CFG.now());
263+
testDateRangeWithCurrentFunctions_AndRangeOptimization("NOW()", DATE_FORMAT,
264+
TestUtils.TEST_CFG.now().minusDays(1L).minusSeconds(1L),
265+
TestUtils.TEST_CFG.now().plusDays(1L).plusSeconds(1L));
254266
}
255267

256268
public void testDateRangeWithCurrentTime() {
257269
testDateRangeWithCurrentFunctions("CURRENT_TIME()", TIME_FORMAT, TestUtils.TEST_CFG.now());
270+
testDateRangeWithCurrentFunctions_AndRangeOptimization("CURRENT_TIME()", TIME_FORMAT,
271+
TestUtils.TEST_CFG.now().minusDays(1L).minusSeconds(1L),
272+
TestUtils.TEST_CFG.now().plusDays(1L).plusSeconds(1L));
258273
}
259274

260275
private void testDateRangeWithCurrentFunctions(String function, String pattern, ZonedDateTime now) {
@@ -292,6 +307,38 @@ private void testDateRangeWithCurrentFunctions(String function, String pattern,
292307
assertEquals(operator.equals("=") || operator.equals("!=") || operator.equals(">="), rq.includeLower());
293308
assertEquals(pattern, rq.format());
294309
}
310+
311+
private void testDateRangeWithCurrentFunctions_AndRangeOptimization(String function, String pattern, ZonedDateTime lowerValue,
312+
ZonedDateTime upperValue) {
313+
String lowerOperator = randomFrom(new String[] {"<", "<="});
314+
String upperOperator = randomFrom(new String[] {">", ">="});
315+
// use both date-only interval (1 DAY) and time-only interval (1 second) to cover CURRENT_TIMESTAMP and TODAY scenarios
316+
String interval = "(INTERVAL 1 DAY + INTERVAL 1 SECOND)";
317+
318+
PhysicalPlan p = optimizeAndPlan("SELECT some.string FROM test WHERE date" + lowerOperator + function + " + " + interval
319+
+ " AND date " + upperOperator + function + " - " + interval);
320+
assertEquals(EsQueryExec.class, p.getClass());
321+
EsQueryExec eqe = (EsQueryExec) p;
322+
assertEquals(1, eqe.output().size());
323+
assertEquals("test.some.string", eqe.output().get(0).qualifiedName());
324+
assertEquals(DataType.TEXT, eqe.output().get(0).dataType());
325+
326+
Query query = eqe.queryContainer().query();
327+
// the range queries optimization should create a single "range" query with "from" and "to" populated with the values
328+
// in the two branches of the AND condition
329+
assertTrue(query instanceof RangeQuery);
330+
RangeQuery rq = (RangeQuery) query;
331+
assertEquals("date", rq.field());
332+
333+
assertEquals(DateFormatter.forPattern(pattern)
334+
.format(upperValue.withNano(DateUtils.getNanoPrecision(null, upperValue.getNano()))), rq.upper());
335+
assertEquals(DateFormatter.forPattern(pattern)
336+
.format(lowerValue.withNano(DateUtils.getNanoPrecision(null, lowerValue.getNano()))), rq.lower());
337+
338+
assertEquals(lowerOperator.equals("<="), rq.includeUpper());
339+
assertEquals(upperOperator.equals(">="), rq.includeLower());
340+
assertEquals(pattern, rq.format());
341+
}
295342

296343
public void testLikeOnInexact() {
297344
LogicalPlan p = plan("SELECT * FROM test WHERE some.string LIKE '%a%'");

0 commit comments

Comments
 (0)