From 020939a9bd5b34c6d540faa8b3a67b740d661be3 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Tue, 22 Oct 2019 10:13:10 +0300 Subject: [PATCH] Add "format" to "range" queries resulted from optimizing a logical AND (#48073) --- .../xpack/sql/planner/QueryTranslator.java | 33 ++++++++++++- .../sql/planner/QueryTranslatorTests.java | 47 +++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java index a3f1f385346e8..25ff9e9879797 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java @@ -108,11 +108,13 @@ import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.util.Check; import org.elasticsearch.xpack.sql.util.DateUtils; +import org.elasticsearch.xpack.sql.util.Holder; import org.elasticsearch.xpack.sql.util.ReflectionUtils; import java.time.OffsetTime; import java.time.Period; import java.time.ZonedDateTime; +import java.time.temporal.TemporalAccessor; import java.util.Arrays; import java.util.LinkedHashMap; import java.util.List; @@ -821,9 +823,36 @@ protected QueryTranslation asQuery(Range r, boolean onAggs) { if (onAggs) { aggFilter = new AggFilter(at.id().toString(), r.asScript()); } else { + Holder lower = new Holder<>(valueOf(r.lower())); + Holder upper = new Holder<>(valueOf(r.upper())); + Holder format = new Holder<>(dateFormat(r.value())); + + // 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 (format.get() == null) { + DateFormatter formatter = null; + if (lower.get() instanceof ZonedDateTime || upper.get() instanceof ZonedDateTime) { + formatter = DateFormatter.forPattern(DATE_FORMAT); + } else if (lower.get() instanceof OffsetTime || upper.get() instanceof OffsetTime) { + formatter = DateFormatter.forPattern(TIME_FORMAT); + } + if (formatter != null) { + // 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. + if (lower.get() instanceof ZonedDateTime || lower.get() instanceof OffsetTime) { + lower.set(formatter.format((TemporalAccessor) lower.get())); + } + if (upper.get() instanceof ZonedDateTime || upper.get() instanceof OffsetTime) { + upper.set(formatter.format((TemporalAccessor) upper.get())); + } + format.set(formatter.pattern()); + } + } + query = handleQuery(r, r.value(), - () -> new RangeQuery(r.source(), nameOf(r.value()), valueOf(r.lower()), r.includeLower(), - valueOf(r.upper()), r.includeUpper(), dateFormat(r.value()))); + () -> new RangeQuery(r.source(), nameOf(r.value()), lower.get(), r.includeLower(), + upper.get(), r.includeUpper(), format.get())); } return new QueryTranslation(query, aggFilter); } else { diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java index 9b78c4791095e..df500411926ae 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java @@ -239,22 +239,37 @@ public void testDateRangeCast() { public void testDateRangeWithCurrentTimestamp() { testDateRangeWithCurrentFunctions("CURRENT_TIMESTAMP()", DATE_FORMAT, TestUtils.TEST_CFG.now()); + testDateRangeWithCurrentFunctions_AndRangeOptimization("CURRENT_TIMESTAMP()", DATE_FORMAT, + TestUtils.TEST_CFG.now().minusDays(1L).minusSeconds(1L), + TestUtils.TEST_CFG.now().plusDays(1L).plusSeconds(1L)); } public void testDateRangeWithCurrentDate() { testDateRangeWithCurrentFunctions("CURRENT_DATE()", DATE_FORMAT, DateUtils.asDateOnly(TestUtils.TEST_CFG.now())); + testDateRangeWithCurrentFunctions_AndRangeOptimization("CURRENT_DATE()", DATE_FORMAT, + DateUtils.asDateOnly(TestUtils.TEST_CFG.now().minusDays(2L)), + DateUtils.asDateOnly(TestUtils.TEST_CFG.now().plusDays(1L))); } public void testDateRangeWithToday() { testDateRangeWithCurrentFunctions("TODAY()", DATE_FORMAT, DateUtils.asDateOnly(TestUtils.TEST_CFG.now())); + testDateRangeWithCurrentFunctions_AndRangeOptimization("TODAY()", DATE_FORMAT, + DateUtils.asDateOnly(TestUtils.TEST_CFG.now().minusDays(2L)), + DateUtils.asDateOnly(TestUtils.TEST_CFG.now().plusDays(1L))); } public void testDateRangeWithNow() { testDateRangeWithCurrentFunctions("NOW()", DATE_FORMAT, TestUtils.TEST_CFG.now()); + testDateRangeWithCurrentFunctions_AndRangeOptimization("NOW()", DATE_FORMAT, + TestUtils.TEST_CFG.now().minusDays(1L).minusSeconds(1L), + TestUtils.TEST_CFG.now().plusDays(1L).plusSeconds(1L)); } public void testDateRangeWithCurrentTime() { testDateRangeWithCurrentFunctions("CURRENT_TIME()", TIME_FORMAT, TestUtils.TEST_CFG.now()); + testDateRangeWithCurrentFunctions_AndRangeOptimization("CURRENT_TIME()", TIME_FORMAT, + TestUtils.TEST_CFG.now().minusDays(1L).minusSeconds(1L), + TestUtils.TEST_CFG.now().plusDays(1L).plusSeconds(1L)); } private void testDateRangeWithCurrentFunctions(String function, String pattern, ZonedDateTime now) { @@ -292,6 +307,38 @@ private void testDateRangeWithCurrentFunctions(String function, String pattern, assertEquals(operator.equals("=") || operator.equals("!=") || operator.equals(">="), rq.includeLower()); assertEquals(pattern, rq.format()); } + + private void testDateRangeWithCurrentFunctions_AndRangeOptimization(String function, String pattern, ZonedDateTime lowerValue, + ZonedDateTime upperValue) { + String lowerOperator = randomFrom(new String[] {"<", "<="}); + String upperOperator = randomFrom(new String[] {">", ">="}); + // use both date-only interval (1 DAY) and time-only interval (1 second) to cover CURRENT_TIMESTAMP and TODAY scenarios + String interval = "(INTERVAL 1 DAY + INTERVAL 1 SECOND)"; + + PhysicalPlan p = optimizeAndPlan("SELECT some.string FROM test WHERE date" + lowerOperator + function + " + " + interval + + " AND date " + upperOperator + function + " - " + interval); + assertEquals(EsQueryExec.class, p.getClass()); + EsQueryExec eqe = (EsQueryExec) p; + assertEquals(1, eqe.output().size()); + assertEquals("test.some.string", eqe.output().get(0).qualifiedName()); + assertEquals(DataType.TEXT, eqe.output().get(0).dataType()); + + Query query = eqe.queryContainer().query(); + // the range queries optimization should create a single "range" query with "from" and "to" populated with the values + // in the two branches of the AND condition + assertTrue(query instanceof RangeQuery); + RangeQuery rq = (RangeQuery) query; + assertEquals("date", rq.field()); + + assertEquals(DateFormatter.forPattern(pattern) + .format(upperValue.withNano(DateUtils.getNanoPrecision(null, upperValue.getNano()))), rq.upper()); + assertEquals(DateFormatter.forPattern(pattern) + .format(lowerValue.withNano(DateUtils.getNanoPrecision(null, lowerValue.getNano()))), rq.lower()); + + assertEquals(lowerOperator.equals("<="), rq.includeUpper()); + assertEquals(upperOperator.equals(">="), rq.includeLower()); + assertEquals(pattern, rq.format()); + } public void testTranslateDateAdd_WhereClause_Painless() { LogicalPlan p = plan("SELECT int FROM test WHERE DATE_ADD('quarter',int, date) > '2018-09-04'::date");