Skip to content
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
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
5 changes: 5 additions & 0 deletions docs/changelog/127355.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 127355
summary: '`text ==` and `text !=` pushdown'
area: ES|QL
type: enhancement
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public void testPushEqualityOnDefaults() throws IOException {
testPushQuery(value, """
FROM test
| WHERE test == "%value"
""", "*:*", true, true);
""", "#test.keyword:%value -_ignored:test.keyword", false, true);
}

public void testPushEqualityOnDefaultsTooBigToPush() throws IOException {
Expand All @@ -64,12 +64,40 @@ public void testPushEqualityOnDefaultsTooBigToPush() throws IOException {
""", "*:*", true, true);
}

/**
* Turns into an {@code IN} which isn't currently pushed.
*/
public void testPushEqualityOnDefaultsOrTooBig() throws IOException {
String value = "v".repeat(between(0, 256));
String tooBig = "a".repeat(between(257, 1000));
testPushQuery(value, """
FROM test
| WHERE test == "%value" OR test == "%tooBig"
""".replace("%tooBig", tooBig), "*:*", true, true);
}

public void testPushEqualityOnDefaultsOrOther() throws IOException {
String value = "v".repeat(between(0, 256));
testPushQuery(value, """
FROM test
| WHERE test == "%value" OR foo == 2
""", "(#test.keyword:%value -_ignored:test.keyword) foo:[2 TO 2]", false, true);
}

public void testPushEqualityOnDefaultsAndOther() throws IOException {
String value = "v".repeat(between(0, 256));
testPushQuery(value, """
FROM test
| WHERE test == "%value" AND foo == 1
""", "#test.keyword:%value -_ignored:test.keyword #foo:[1 TO 1]", false, true);
}

public void testPushInequalityOnDefaults() throws IOException {
String value = "v".repeat(between(0, 256));
testPushQuery(value, """
FROM test
| WHERE test != "%different_value"
""", "*:*", true, true);
""", "(-test.keyword:%different_value #*:*) _ignored:test.keyword", true, true);
}

public void testPushInequalityOnDefaultsTooBigToPush() throws IOException {
Expand All @@ -91,7 +119,7 @@ public void testPushCaseInsensitiveEqualityOnDefaults() throws IOException {
private void testPushQuery(String value, String esqlQuery, String luceneQuery, boolean filterInCompute, boolean found)
throws IOException {
indexValue(value);
String differentValue = randomValueOtherThan(value, () -> randomAlphaOfLength(value.length() == 0 ? 1 : value.length()));
String differentValue = randomValueOtherThan(value, () -> randomAlphaOfLength(value.isEmpty() ? 1 : value.length()));

String replacedQuery = esqlQuery.replaceAll("%value", value).replaceAll("%different_value", differentValue);
RestEsqlTestCase.RequestObjectBuilder builder = requestObjectBuilder().query(replacedQuery + "\n| KEEP test");
Expand Down Expand Up @@ -167,7 +195,7 @@ private void indexValue(String value) throws IOException {
bulk.addParameter("refresh", "");
bulk.setJsonEntity(String.format(Locale.ROOT, """
{"create":{"_index":"test"}}
{"test":"%s"}
{"test":"%s","foo":1}
""", value));
Response bulkResponse = client().performRequest(bulk);
assertThat(entityToMap(bulkResponse.getEntity(), XContentType.JSON), matchesMap().entry("errors", false).extraOk());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@
2023-10-23T13:55:01.544Z,Connected to 10.1.0.1
2023-10-23T13:55:01.545Z,[Connected to 10.1.0.1, More than one hundred characters long so it isn't indexed by the sub keyword field with ignore_above:100]
2023-10-23T13:55:01.546Z,More than one hundred characters long so it isn't indexed by the sub keyword field with ignore_above:100
2023-10-23T13:55:01.547Z,[More than one hundred characters long so it isn't indexed by the sub keyword field with ignore_above:100,Second than one hundred characters long so it isn't indexed by the sub keyword field with ignore_above:100]
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,34 @@

package org.elasticsearch.xpack.esql.capabilities;

import org.elasticsearch.compute.lucene.LuceneTopNSourceOperator;
import org.elasticsearch.compute.operator.FilterOperator;
import org.elasticsearch.xpack.esql.core.expression.Expression;
import org.elasticsearch.xpack.esql.core.querydsl.query.Query;
import org.elasticsearch.xpack.esql.optimizer.rules.physical.local.LucenePushdownPredicates;
import org.elasticsearch.xpack.esql.planner.TranslatorHandler;

/**
* Expressions implementing this interface can get called on data nodes to provide an Elasticsearch/Lucene query.
* Expressions implementing this interface are asked provide an
* Elasticsearch/Lucene query on the as part of the data node optimizations.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on the is probably a leftover

*/
public interface TranslationAware {
/**
* Indicates whether the expression can be translated or not.
* Usually checks whether the expression arguments are actual fields that exist in Lucene.
* Can this instance be translated or not? Usually checks whether the
* expression arguments are actual fields that exist in Lucene. See {@link Translatable}
* for precisely what can be signaled from this method.
*/
boolean translatable(LucenePushdownPredicates pushdownPredicates);
Translatable translatable(LucenePushdownPredicates pushdownPredicates);

/**
* Is an {@link Expression} translatable?
*/
static TranslationAware.Translatable translatable(Expression exp, LucenePushdownPredicates lucenePushdownPredicates) {
if (exp instanceof TranslationAware aware) {
return aware.translatable(lucenePushdownPredicates);
}
return TranslationAware.Translatable.NO;
}

/**
* Translates the implementing expression into a Query.
Expand All @@ -42,4 +56,112 @@ interface SingleValueTranslationAware extends TranslationAware {
*/
Expression singleValueField();
}

/**
* How is this expression translatable?
*/
enum Translatable {
/**
* Not translatable at all. Calling {@link TranslationAware#asQuery} is an error.
* The expression will stay in the query plan and be filtered via a {@link FilterOperator}.
* Imagine {@code kwd == "a"} when {@code kwd} is configured without a search index.
*/
NO(FinishedTranslatable.NO),
/**
* Entirely translatable into a lucene query. Calling {@link TranslationAware#asQuery}
* will produce a query that matches all documents matching this expression and
* <strong>only</strong> documents matching this expression. Imagine {@code kwd == "a"}
* when {@code kwd} has a search index and doc values - which is the
* default configuration. This will entirely remove the clause from the
* {@code WHERE}, removing the entire {@link FilterOperator} if it's empty. Sometimes
* this allows us to push the entire top-n operation to lucene with
* a {@link LuceneTopNSourceOperator}.
*/
YES(FinishedTranslatable.YES),
/**
* Translation requires a recheck. Calling {@link TranslationAware#asQuery} will
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

* produce a query that matches all documents matching this expression but might
* match more documents that do not match the expression. This will cause us to
* push a query to lucene <strong>and</strong> keep the query in the query plan,
* rechecking it via a {@link FilterOperator}. This can never push the entire
* top-n to Lucene, but it's still quite a lot better than the full scan from
* {@link #NO}.
* <p>
* Imagine {@code kwd == "a"} where {@code kwd} has a search index but doesn't
* have doc values. In that case we can find candidate matches in lucene but
* can't tell if those docs are single-valued. If they are multivalued they'll
* still match the query but won't match the expression. Thus, the double-checking.
* <strong>Technically</strong> we could just check for single-valued-ness in
* this case, but it's simpler to
* </p>
*/
RECHECK(FinishedTranslatable.RECHECK),
/**
* The same as {@link #YES}, but if this expression is negated it turns into {@link #RECHECK}.
* This comes up when pushing {@code NOT(text == "a")} to {@code text.keyword} which can
* have ignored fields.
*/
YES_BUT_RECHECK_NEGATED(FinishedTranslatable.YES);

private final FinishedTranslatable finish;

Translatable(FinishedTranslatable finish) {
this.finish = finish;
}

/**
* Translate into a {@link FinishedTranslatable} which never
* includes {@link #YES_BUT_RECHECK_NEGATED}.
*/
public FinishedTranslatable finish() {
return finish;
}

public Translatable negate() {
if (this == YES_BUT_RECHECK_NEGATED) {
return RECHECK;
}
return this;
}

/**
* Merge two {@link TranslationAware#translatable} results.
*/
public Translatable merge(Translatable rhs) {
return switch (this) {
case NO -> NO;
case YES -> switch (rhs) {
case NO -> NO;
case YES -> YES;
case RECHECK -> RECHECK;
case YES_BUT_RECHECK_NEGATED -> YES_BUT_RECHECK_NEGATED;
};
case RECHECK -> switch (rhs) {
case NO -> NO;
case YES, RECHECK, YES_BUT_RECHECK_NEGATED -> RECHECK;
};
case YES_BUT_RECHECK_NEGATED -> switch (rhs) {
case NO -> NO;
case YES, YES_BUT_RECHECK_NEGATED -> YES_BUT_RECHECK_NEGATED;
case RECHECK -> RECHECK;
};
};
}

}

enum FinishedTranslatable {
/**
* See {@link Translatable#YES}.
*/
YES,
/**
* See {@link Translatable#NO}.
*/
NO,
/**
* See {@link Translatable#RECHECK}.
*/
RECHECK;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,9 @@ public boolean equals(Object obj) {
}

@Override
public boolean translatable(LucenePushdownPredicates pushdownPredicates) {
public Translatable translatable(LucenePushdownPredicates pushdownPredicates) {
// In isolation, full text functions are pushable to source. We check if there are no disjunctions in Or conditions
return true;
return Translatable.YES;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,8 @@ protected NodeInfo<? extends Expression> info() {
}

@Override
public boolean translatable(LucenePushdownPredicates pushdownPredicates) {
return pushdownPredicates.isPushableFieldAttribute(ipField) && Expressions.foldable(matches);
public Translatable translatable(LucenePushdownPredicates pushdownPredicates) {
return pushdownPredicates.isPushableFieldAttribute(ipField) && Expressions.foldable(matches) ? Translatable.YES : Translatable.NO;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.geometry.Geometry;
import org.elasticsearch.lucene.spatial.CoordinateEncoder;
import org.elasticsearch.xpack.esql.capabilities.TranslationAware;
import org.elasticsearch.xpack.esql.core.expression.Expression;
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
import org.elasticsearch.xpack.esql.core.expression.TypeResolutions;
Expand Down Expand Up @@ -273,12 +274,14 @@ protected Geometry fromBytesRef(BytesRef bytesRef) {
/**
* Push-down to Lucene is only possible if one field is an indexed spatial field, and the other is a constant spatial or string column.
*/
public boolean translatable(LucenePushdownPredicates pushdownPredicates) {
public TranslationAware.Translatable translatable(LucenePushdownPredicates pushdownPredicates) {
// The use of foldable here instead of SpatialEvaluatorFieldKey.isConstant is intentional to match the behavior of the
// Lucene pushdown code in EsqlTranslationHandler::SpatialRelatesTranslator
// We could enhance both places to support ReferenceAttributes that refer to constants, but that is a larger change
return isPushableSpatialAttribute(left(), pushdownPredicates) && right().foldable()
|| isPushableSpatialAttribute(right(), pushdownPredicates) && left().foldable();
|| isPushableSpatialAttribute(right(), pushdownPredicates) && left().foldable()
? TranslationAware.Translatable.YES
: TranslationAware.Translatable.NO;

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ protected void processPointDocValuesAndSource(
}

@Override
public boolean translatable(LucenePushdownPredicates pushdownPredicates) {
public Translatable translatable(LucenePushdownPredicates pushdownPredicates) {
return super.translatable(pushdownPredicates); // only for the explicit Override, as only this subclass implements TranslationAware
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,8 @@ public ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {
}

@Override
public boolean translatable(LucenePushdownPredicates pushdownPredicates) {
return pushdownPredicates.isPushableAttribute(str) && suffix.foldable();
public Translatable translatable(LucenePushdownPredicates pushdownPredicates) {
return pushdownPredicates.isPushableAttribute(str) && suffix.foldable() ? Translatable.YES : Translatable.NO;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ public EvalOperator.ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvalua
}

@Override
public boolean translatable(LucenePushdownPredicates pushdownPredicates) {
return pushdownPredicates.isPushableFieldAttribute(field());
public Translatable translatable(LucenePushdownPredicates pushdownPredicates) {
return pushdownPredicates.isPushableFieldAttribute(field()) ? Translatable.YES : Translatable.NO;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ public ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {
}

@Override
public boolean translatable(LucenePushdownPredicates pushdownPredicates) {
return pushdownPredicates.isPushableAttribute(str) && prefix.foldable();
public Translatable translatable(LucenePushdownPredicates pushdownPredicates) {
return pushdownPredicates.isPushableAttribute(str) && prefix.foldable() ? Translatable.YES : Translatable.NO;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,8 @@ public EvalOperator.ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvalua
}

@Override
public boolean translatable(LucenePushdownPredicates pushdownPredicates) {
return pushdownPredicates.isPushableAttribute(field());
public Translatable translatable(LucenePushdownPredicates pushdownPredicates) {
return pushdownPredicates.isPushableAttribute(field()) ? Translatable.YES : Translatable.NO;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,8 @@ public boolean equals(Object obj) {
}

@Override
public boolean translatable(LucenePushdownPredicates pushdownPredicates) {
return pushdownPredicates.isPushableAttribute(value) && lower.foldable() && upper.foldable();
public Translatable translatable(LucenePushdownPredicates pushdownPredicates) {
return pushdownPredicates.isPushableAttribute(value) && lower.foldable() && upper.foldable() ? Translatable.YES : Translatable.NO;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,8 @@ protected boolean isCommutative() {
}

@Override
public boolean translatable(LucenePushdownPredicates pushdownPredicates) {
return left() instanceof TranslationAware leftAware
&& leftAware.translatable(pushdownPredicates)
&& right() instanceof TranslationAware rightAware
&& rightAware.translatable(pushdownPredicates);
public Translatable translatable(LucenePushdownPredicates pushdownPredicates) {
return TranslationAware.translatable(left(), pushdownPredicates).merge(TranslationAware.translatable(right(), pushdownPredicates));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ static Expression negate(Expression exp) {
}

@Override
public boolean translatable(LucenePushdownPredicates pushdownPredicates) {
return field() instanceof TranslationAware aware && aware.translatable(pushdownPredicates);
public Translatable translatable(LucenePushdownPredicates pushdownPredicates) {
return TranslationAware.translatable(field(), pushdownPredicates).negate();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public UnaryScalarFunction negate() {
}

@Override
public boolean translatable(LucenePushdownPredicates pushdownPredicates) {
public Translatable translatable(LucenePushdownPredicates pushdownPredicates) {
return IsNull.isTranslatable(field(), pushdownPredicates);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,14 @@ public UnaryScalarFunction negate() {
}

@Override
public boolean translatable(LucenePushdownPredicates pushdownPredicates) {
public Translatable translatable(LucenePushdownPredicates pushdownPredicates) {
return isTranslatable(field(), pushdownPredicates);
}

protected static boolean isTranslatable(Expression field, LucenePushdownPredicates pushdownPredicates) {
return LucenePushdownPredicates.isPushableTextFieldAttribute(field) || pushdownPredicates.isPushableFieldAttribute(field);
protected static Translatable isTranslatable(Expression field, LucenePushdownPredicates pushdownPredicates) {
return LucenePushdownPredicates.isPushableTextFieldAttribute(field) || pushdownPredicates.isPushableFieldAttribute(field)
? Translatable.YES
: Translatable.NO;
}

@Override
Expand Down
Loading
Loading