Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ES|QL] Create Range in PushFiltersToSource for qualified pushable filters on the same field #111437

Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
2422f70
add CombineBinaryComparisons
fang-xing-esql Jul 5, 2024
7c58b68
Update docs/changelog/110548.yaml
fang-xing-esql Jul 5, 2024
d321d1d
Merge branch 'main' into CombineBinaryComparisons
fang-xing-esql Jul 17, 2024
277c48f
add more tests from OptimizerRulesTests
fang-xing-esql Jul 17, 2024
75e4140
EsqlCapabilities
fang-xing-esql Jul 18, 2024
925a87b
Merge branch 'main' into CombineBinaryComparisons
fang-xing-esql Jul 18, 2024
d36adae
more test as suggested
fang-xing-esql Jul 18, 2024
d5b2112
Merge branch 'main' into CombineBinaryComparisons
fang-xing-esql Jul 24, 2024
81892c0
Merge branch 'main' into CombineBinaryComparisons
fang-xing-esql Jul 24, 2024
5bcf8a4
Merge branch 'main' into CombineBinaryComparisons
fang-xing-esql Jul 29, 2024
3e520a7
CombineBinaryComparisons with Range pushed down into lucene
fang-xing-esql Jul 29, 2024
1adb697
Update docs/changelog/111437.yaml
fang-xing-esql Jul 30, 2024
3fa0bfd
Merge branch 'main' into CombineBinaryComparisonsWithRangeQuery
fang-xing-esql Jul 30, 2024
aa8e9a1
Update docs/changelog/111437.yaml
fang-xing-esql Jul 31, 2024
542fc13
Merge branch 'main' into CombineBinaryComparisonsWithRangeQuery
fang-xing-esql Aug 5, 2024
e422964
clean up
fang-xing-esql Aug 5, 2024
8ad165b
Merge branch 'main' into CombineBinaryComparisonsWithRangeQuery
fang-xing-esql Aug 6, 2024
1ab0d54
update according to review comments
fang-xing-esql Aug 6, 2024
3d6fe5c
Merge branch 'main' into CombineBinaryComparisonsWithRangeQuery
fang-xing-esql Aug 8, 2024
0d164f8
address review comments
fang-xing-esql Aug 8, 2024
713ea72
Merge branch 'main' into CombineBinaryComparisonsWithRangeQuery
fang-xing-esql Aug 8, 2024
b7eb9e0
update according to review comments
fang-xing-esql Aug 9, 2024
26e5976
more rest tests
fang-xing-esql Aug 9, 2024
295d451
Merge branch 'main' into CombineBinaryComparisonsWithRangeQuery
fang-xing-esql Aug 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions docs/changelog/110548.yaml

This file was deleted.

5 changes: 5 additions & 0 deletions docs/changelog/111437.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 111437
summary: "[ES|QL] Add `CombineBinaryComparisons` with range query"
fang-xing-esql marked this conversation as resolved.
Show resolved Hide resolved
area: ES|QL
type: enhancement
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,10 @@

package org.elasticsearch.xpack.esql.core.planner;

import org.elasticsearch.common.time.DateFormatter;
import org.elasticsearch.xpack.esql.core.QlIllegalArgumentException;
import org.elasticsearch.xpack.esql.core.expression.Expression;
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
import org.elasticsearch.xpack.esql.core.expression.MetadataAttribute;
import org.elasticsearch.xpack.esql.core.expression.predicate.Range;
import org.elasticsearch.xpack.esql.core.expression.predicate.fulltext.MatchQueryPredicate;
import org.elasticsearch.xpack.esql.core.expression.predicate.fulltext.MultiMatchQueryPredicate;
import org.elasticsearch.xpack.esql.core.expression.predicate.fulltext.StringQueryPredicate;
Expand All @@ -32,31 +30,17 @@
import org.elasticsearch.xpack.esql.core.querydsl.query.NotQuery;
import org.elasticsearch.xpack.esql.core.querydsl.query.Query;
import org.elasticsearch.xpack.esql.core.querydsl.query.QueryStringQuery;
import org.elasticsearch.xpack.esql.core.querydsl.query.RangeQuery;
import org.elasticsearch.xpack.esql.core.querydsl.query.RegexQuery;
import org.elasticsearch.xpack.esql.core.querydsl.query.WildcardQuery;
import org.elasticsearch.xpack.esql.core.tree.Source;
import org.elasticsearch.xpack.esql.core.util.Check;
import org.elasticsearch.xpack.esql.core.util.CollectionUtils;

import java.time.OffsetTime;
import java.time.ZonedDateTime;
import java.time.temporal.TemporalAccessor;
import java.util.Arrays;
import java.util.List;

public final class ExpressionTranslators {

public static final String DATE_FORMAT = "strict_date_optional_time_nanos";
public static final String TIME_FORMAT = "strict_hour_minute_second_fraction";

public static Object valueOf(Expression e) {
if (e.foldable()) {
return e.fold();
}
throw new QlIllegalArgumentException("Cannot determine value for {}", e);
}

// TODO: see whether escaping is needed
@SuppressWarnings("rawtypes")
public static class Likes extends ExpressionTranslator<RegexMatch> {
Expand Down Expand Up @@ -193,55 +177,6 @@ private static Query translate(IsNull isNull, TranslatorHandler handler) {
}
}

public static class Ranges extends ExpressionTranslator<Range> {

@Override
protected Query asQuery(Range r, TranslatorHandler handler) {
return doTranslate(r, handler);
}

public static Query doTranslate(Range r, TranslatorHandler handler) {
return handler.wrapFunctionQuery(r, r.value(), () -> translate(r, handler));
}

private static RangeQuery translate(Range r, TranslatorHandler handler) {
Object lower = valueOf(r.lower());
Object upper = valueOf(r.upper());
String format = null;

// 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
DateFormatter formatter = null;
if (lower instanceof ZonedDateTime || upper instanceof ZonedDateTime) {
formatter = DateFormatter.forPattern(DATE_FORMAT);
} else if (lower instanceof OffsetTime || upper 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 instanceof ZonedDateTime || lower instanceof OffsetTime) {
lower = formatter.format((TemporalAccessor) lower);
}
if (upper instanceof ZonedDateTime || upper instanceof OffsetTime) {
upper = formatter.format((TemporalAccessor) upper);
}
format = formatter.pattern();
}
return new RangeQuery(
r.source(),
handler.nameOf(r.value()),
lower,
r.includeLower(),
upper,
r.includeUpper(),
format,
r.zoneId()
);
}
}

public static Query or(Source source, Query left, Query right) {
return boolQuery(source, left, right, false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

import java.io.IOException;

import static org.elasticsearch.xpack.esql.core.planner.ExpressionTranslators.valueOf;
import static org.elasticsearch.xpack.esql.core.expression.Foldables.valueOf;

public class SpatialRelatesUtils {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.elasticsearch.xpack.esql.core.expression.TypedAttribute;
import org.elasticsearch.xpack.esql.core.expression.function.scalar.UnaryScalarFunction;
import org.elasticsearch.xpack.esql.core.expression.predicate.Predicates;
import org.elasticsearch.xpack.esql.core.expression.predicate.Range;
import org.elasticsearch.xpack.esql.core.expression.predicate.fulltext.MatchQueryPredicate;
import org.elasticsearch.xpack.esql.core.expression.predicate.fulltext.StringQueryPredicate;
import org.elasticsearch.xpack.esql.core.expression.predicate.logical.And;
Expand All @@ -45,6 +46,7 @@
import org.elasticsearch.xpack.esql.core.rule.Rule;
import org.elasticsearch.xpack.esql.core.tree.Source;
import org.elasticsearch.xpack.esql.core.type.DataType;
import org.elasticsearch.xpack.esql.core.util.CollectionUtils;
import org.elasticsearch.xpack.esql.core.util.Queries;
import org.elasticsearch.xpack.esql.core.util.Queries.Clause;
import org.elasticsearch.xpack.esql.core.util.StringUtils;
Expand All @@ -59,8 +61,12 @@
import org.elasticsearch.xpack.esql.expression.function.scalar.spatial.StDistance;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.Equals;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.EsqlBinaryComparison;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.GreaterThan;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.GreaterThanOrEqual;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.In;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.InsensitiveBinaryComparison;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.LessThan;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.LessThanOrEqual;
import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.NotEquals;
import org.elasticsearch.xpack.esql.optimizer.PhysicalOptimizerRules.OptimizerRule;
import org.elasticsearch.xpack.esql.plan.physical.AggregateExec;
Expand Down Expand Up @@ -252,8 +258,10 @@ protected PhysicalPlan rule(FilterExec filterExec, LocalPhysicalOptimizerContext
for (Expression exp : splitAnd(filterExec.condition())) {
(canPushToSource(exp, x -> hasIdenticalDelegate(x, ctx.searchStats())) ? pushable : nonPushable).add(exp);
}
if (pushable.size() > 0) { // update the executable with pushable conditions
Query queryDSL = TRANSLATOR_HANDLER.asQuery(Predicates.combineAnd(pushable));
// Combine GT, GTE, LT and LTE in pushable to Range if possible
List<Expression> newPushable = combineEligiblePushableToRange(pushable);
if (newPushable.size() > 0) { // update the executable with pushable conditions
Query queryDSL = TRANSLATOR_HANDLER.asQuery(Predicates.combineAnd(newPushable));
QueryBuilder planQuery = queryDSL.asBuilder();
var query = Queries.combine(Clause.FILTER, asList(queryExec.query(), planQuery));
queryExec = new EsQueryExec(
Expand All @@ -277,6 +285,79 @@ protected PhysicalPlan rule(FilterExec filterExec, LocalPhysicalOptimizerContext
return plan;
}

private static List<Expression> combineEligiblePushableToRange(List<Expression> pushable) {
List<EsqlBinaryComparison> bcs = new ArrayList<>();
List<Range> ranges = new ArrayList<>();
List<Expression> others = new ArrayList<>();
boolean changed = false;

pushable.forEach(e -> {
if (e instanceof GreaterThan || e instanceof GreaterThanOrEqual || e instanceof LessThan || e instanceof LessThanOrEqual) {
if (((EsqlBinaryComparison) e).right().foldable()) {
bcs.add((EsqlBinaryComparison) e);
} else {
others.add(e);
}
} else {
others.add(e);
}
});

for (int i = 0, step = 1; i < bcs.size() - 1; i += step, step = 1) {
BinaryComparison main = bcs.get(i);
for (int j = i + 1; j < bcs.size(); j++) {
fang-xing-esql marked this conversation as resolved.
Show resolved Hide resolved
BinaryComparison other = bcs.get(j);
if (main.left().semanticEquals(other.left())) {
// >/>= AND </<=
if ((main instanceof GreaterThan || main instanceof GreaterThanOrEqual)
&& (other instanceof LessThan || other instanceof LessThanOrEqual)) {
bcs.remove(j);
bcs.remove(i);

ranges.add(
new Range(
main.source(),
main.left(),
main.right(),
main instanceof GreaterThanOrEqual,
other.right(),
other instanceof LessThanOrEqual,
main.zoneId()
)
);

changed = true;
step = 0;
break;
}
// </<= AND >/>=
else if ((other instanceof GreaterThan || other instanceof GreaterThanOrEqual)
&& (main instanceof LessThan || main instanceof LessThanOrEqual)) {
bcs.remove(j);
bcs.remove(i);

ranges.add(
new Range(
main.source(),
main.left(),
other.right(),
other instanceof GreaterThanOrEqual,
main.right(),
main instanceof LessThanOrEqual,
main.zoneId()
)
);

changed = true;
step = 0;
break;
}
}
}
}
return changed ? CollectionUtils.combine(others, bcs, ranges) : pushable;
}

public static boolean canPushToSource(Expression exp, Predicate<FieldAttribute> hasIdenticalDelegate) {
if (exp instanceof BinaryComparison bc) {
return isAttributePushable(bc.left(), bc, hasIdenticalDelegate) && bc.right().foldable();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
import org.elasticsearch.xpack.esql.core.expression.TypedAttribute;
import org.elasticsearch.xpack.esql.core.expression.function.scalar.ScalarFunction;
import org.elasticsearch.xpack.esql.core.expression.predicate.Range;
import org.elasticsearch.xpack.esql.core.expression.predicate.operator.comparison.BinaryComparison;
import org.elasticsearch.xpack.esql.core.planner.ExpressionTranslator;
import org.elasticsearch.xpack.esql.core.planner.ExpressionTranslators;
Expand Down Expand Up @@ -50,11 +51,13 @@
import java.time.OffsetTime;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.time.temporal.TemporalAccessor;
import java.util.ArrayList;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;

import static org.elasticsearch.xpack.esql.core.expression.Foldables.valueOf;
import static org.elasticsearch.xpack.esql.core.planner.ExpressionTranslators.or;
import static org.elasticsearch.xpack.esql.core.type.DataType.IP;
import static org.elasticsearch.xpack.esql.core.type.DataType.UNSIGNED_LONG;
Expand All @@ -67,14 +70,15 @@
import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.versionToString;

public final class EsqlExpressionTranslators {

public static final List<ExpressionTranslator<?>> QUERY_TRANSLATORS = List.of(
new EqualsIgnoreCaseTranslator(),
new BinaryComparisons(),
new SpatialRelatesTranslator(),
new InComparisons(),
// Ranges is redundant until we start combining binary comparisons (see CombineBinaryComparisons in ql's OptimizerRules)
// or introduce a BETWEEN keyword.
fang-xing-esql marked this conversation as resolved.
Show resolved Hide resolved
new ExpressionTranslators.Ranges(),
new Ranges(),
new ExpressionTranslators.BinaryLogic(),
new ExpressionTranslators.IsNulls(),
new ExpressionTranslators.IsNotNulls(),
Expand Down Expand Up @@ -124,7 +128,7 @@ public static void checkInsensitiveComparison(InsensitiveEquals bc) {
static Query translate(InsensitiveEquals bc) {
TypedAttribute attribute = checkIsPushableAttribute(bc.left());
Source source = bc.source();
BytesRef value = BytesRefs.toBytesRef(ExpressionTranslators.valueOf(bc.right()));
BytesRef value = BytesRefs.toBytesRef(valueOf(bc.right()));
String name = pushableAttributeName(attribute);
return new TermQuery(source, name, value.utf8ToString(), true);
}
Expand Down Expand Up @@ -249,7 +253,7 @@ private static Query translateOutOfRangeComparisons(BinaryComparison bc) {
return null;
}
Source source = bc.source();
Object value = ExpressionTranslators.valueOf(bc.right());
Object value = valueOf(bc.right());

// Comparisons with multi-values always return null in ESQL.
if (value instanceof List<?>) {
Expand Down Expand Up @@ -453,12 +457,90 @@ private static Query translate(In in, TranslatorHandler handler) {

return queries.stream().reduce((q1, q2) -> or(in.source(), q1, q2)).get();
}
}

public static class Ranges extends ExpressionTranslator<Range> {

@Override
protected Query asQuery(Range r, TranslatorHandler handler) {
return doTranslate(r, handler);
}

public static Query doTranslate(Range r, TranslatorHandler handler) {
return handler.wrapFunctionQuery(r, r.value(), () -> translate(r, handler));
}

private static RangeQuery translate(Range r, TranslatorHandler handler) {
Object lower = valueOf(r.lower());
Object upper = valueOf(r.upper());
String format = null;

// 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
DateFormatter formatter = null;
if (lower instanceof ZonedDateTime || upper instanceof ZonedDateTime) {
formatter = DEFAULT_DATE_TIME_FORMATTER;
} else if (lower instanceof OffsetTime || upper instanceof OffsetTime) {
formatter = HOUR_MINUTE_SECOND;
fang-xing-esql marked this conversation as resolved.
Show resolved Hide resolved
}
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 instanceof ZonedDateTime || lower instanceof OffsetTime) {
lower = formatter.format((TemporalAccessor) lower);
}
if (upper instanceof ZonedDateTime || upper instanceof OffsetTime) {
upper = formatter.format((TemporalAccessor) upper);
}
format = formatter.pattern();
}

public static Object valueOf(Expression e) {
if (e.foldable()) {
return e.fold();
DataType dataType = r.value().dataType();
if (DataType.isDateTime(dataType) && DataType.isDateTime(r.lower().dataType()) && DataType.isDateTime(r.upper().dataType())) {
lower = dateTimeToString((Long) lower);
upper = dateTimeToString((Long) upper);
format = DEFAULT_DATE_TIME_FORMATTER.pattern();
}
throw new QlIllegalArgumentException("Cannot determine value for {}", e);

if (dataType == IP) {
if (lower instanceof BytesRef bytesRef) {
lower = ipToString(bytesRef);
}
if (upper instanceof BytesRef bytesRef) {
upper = ipToString(bytesRef);
}
} else if (dataType == VERSION) {
// VersionStringFieldMapper#indexedValueForSearch() only accepts as input String or BytesRef with the String (i.e. not
// encoded) representation of the version as it'll do the encoding itself.
if (lower instanceof BytesRef bytesRef) {
lower = versionToString(bytesRef);
} else if (lower instanceof Version version) {
lower = versionToString(version);
}
if (upper instanceof BytesRef bytesRef) {
upper = versionToString(bytesRef);
} else if (upper instanceof Version version) {
upper = versionToString(version);
}
} else if (dataType == UNSIGNED_LONG) {
if (lower instanceof Long ul) {
lower = unsignedLongAsNumber(ul);
}
if (upper instanceof Long ul) {
upper = unsignedLongAsNumber(ul);
}
}
return new RangeQuery(
r.source(),
handler.nameOf(r.value()),
lower,
r.includeLower(),
upper,
r.includeUpper(),
format,
r.zoneId()
);
}
}
}
Loading