Skip to content

Commit

Permalink
Add "format" to "range" queries resulted from optimizing a logical AND (
Browse files Browse the repository at this point in the history 
#48073)
  • Loading branch information
@astefan
astefan authored Oct 22, 2019
1 parent c1c4efc commit 020939a
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 2 deletions.
33 changes: 31 additions & 2 deletions x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -821,9 +823,36 @@ protected QueryTranslation asQuery(Range r, boolean onAggs) {
if (onAggs) {
aggFilter = new AggFilter(at.id().toString(), r.asScript());
} else {
Holder<Object> lower = new Holder<>(valueOf(r.lower()));
Holder<Object> upper = new Holder<>(valueOf(r.upper()));
Holder<String> 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 {
Expand Down
47 changes: 47 additions & 0 deletions ...ck/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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");
Expand Down

0 comments on commit 020939a

Please sign in to comment.