Skip to content

Commit

Permalink
QL: Turn eager query translations lazy
Browse files Browse the repository at this point in the history 
Before the `ExpressionTranslators` did some unnecessary
`Expression` -> `Query` translations, where the result queries were thrown
away by later translations (by the `wrapFunctionQuery`).

This change adds laziness, so translations don't execute unnecessarily.

Follows #68783
  • Loading branch information
Andras Palinkas committed Feb 18, 2021
1 parent 55f0e32 commit 6035c31
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 25 deletions.
4 changes: 2 additions & 2 deletions x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/planner/QueryTranslator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ protected Query asQuery(InsensitiveBinaryComparison bc, TranslatorHandler handle

public static Query doTranslate(InsensitiveBinaryComparison bc, TranslatorHandler handler) {
checkInsensitiveComparison(bc);
return handler.wrapFunctionQuery(bc, bc.left(), translate(bc, handler));
return handler.wrapFunctionQuery(bc, bc.left(), () -> translate(bc, handler));
}

public static void checkInsensitiveComparison(InsensitiveBinaryComparison bc) {
Expand Down Expand Up @@ -163,7 +163,7 @@ public static Query doTranslate(ScalarFunction f, TranslatorHandler handler) {
}
}

return handler.wrapFunctionQuery(f, f, new ScriptQuery(f.source(), f.asScript()));
return handler.wrapFunctionQuery(f, f, () -> new ScriptQuery(f.source(), f.asScript()));
}
}

Expand Down
42 changes: 26 additions & 16 deletions x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/ExpressionTranslators.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,15 +216,17 @@ protected Query asQuery(IsNotNull isNotNull, TranslatorHandler handler) {
}

public static Query doTranslate(IsNotNull isNotNull, TranslatorHandler handler) {
Query query = null;
return handler.wrapFunctionQuery(isNotNull, isNotNull.field(), () -> translate(isNotNull, handler));
}

private static Query translate(IsNotNull isNotNull, TranslatorHandler handler) {
Query query = null;
if (isNotNull.field() instanceof FieldAttribute) {
query = new ExistsQuery(isNotNull.source(), handler.nameOf(isNotNull.field()));
} else {
query = new ScriptQuery(isNotNull.source(), isNotNull.asScript());
}

return handler.wrapFunctionQuery(isNotNull, isNotNull.field(), query);
return query;
}
}

Expand All @@ -236,6 +238,10 @@ protected Query asQuery(IsNull isNull, TranslatorHandler handler) {
}

public static Query doTranslate(IsNull isNull, TranslatorHandler handler) {
return handler.wrapFunctionQuery(isNull, isNull.field(), () -> translate(isNull, handler));
}

private static Query translate(IsNull isNull, TranslatorHandler handler) {
Query query = null;

if (isNull.field() instanceof FieldAttribute) {
Expand All @@ -244,7 +250,7 @@ public static Query doTranslate(IsNull isNull, TranslatorHandler handler) {
query = new ScriptQuery(isNull.source(), isNull.asScript());
}

return handler.wrapFunctionQuery(isNull, isNull.field(), query);
return query;
}
}

Expand All @@ -265,7 +271,7 @@ public static void checkBinaryComparison(BinaryComparison bc) {

public static Query doTranslate(BinaryComparison bc, TranslatorHandler handler) {
checkBinaryComparison(bc);
return handler.wrapFunctionQuery(bc, bc.left(), translate(bc, handler));
return handler.wrapFunctionQuery(bc, bc.left(), () -> translate(bc, handler));
}

private static Query translate(BinaryComparison bc, TranslatorHandler handler) {
Expand Down Expand Up @@ -340,10 +346,11 @@ protected Query asQuery(Range r, TranslatorHandler handler) {
return doTranslate(r, handler);
}

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

Query query = null;
private static RangeQuery translate(Range r, TranslatorHandler handler) {
Object lower = valueOf(r.lower());
Object upper = valueOf(r.upper());
String format = null;
Expand All @@ -368,10 +375,8 @@ public static Query doTranslate(Range r, TranslatorHandler handler) {
}
format = formatter.pattern();
}
query = handler.wrapFunctionQuery(r, val,
new RangeQuery(r.source(), handler.nameOf(val), lower, r.includeLower(), upper, r.includeUpper(), format, r.zoneId()));

return query;
return new RangeQuery(
r.source(), handler.nameOf(r.value()), lower, r.includeLower(), upper, r.includeUpper(), format, r.zoneId());
}
}

Expand All @@ -383,6 +388,10 @@ protected Query asQuery(In in, TranslatorHandler handler) {
}

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

private static Query translate(In in, TranslatorHandler handler) {
Query q;
if (in.value() instanceof FieldAttribute) {
// equality should always be against an exact match (which is important for strings)
Expand All @@ -406,8 +415,9 @@ public static Query doTranslate(In in, TranslatorHandler handler) {
assert o instanceof ZonedDateTime : "expected a ZonedDateTime, but got: " + o.getClass().getName();
// see comment in Ranges#doTranslate() as to why formatting as String is required
String zdt = formatter.format((ZonedDateTime) o);
RangeQuery right = new RangeQuery(in.source(), fa.exactAttribute().name(),
zdt, true, zdt, true, formatter.pattern(), in.zoneId());
RangeQuery right = new RangeQuery(
in.source(), fa.exactAttribute().name(),
zdt, true, zdt, true, formatter.pattern(), in.zoneId());
q = q == null ? right : new BoolQuery(in.source(), false, q, right);
}
} else {
Expand All @@ -416,7 +426,7 @@ public static Query doTranslate(In in, TranslatorHandler handler) {
} else {
q = new ScriptQuery(in.source(), in.asScript());
}
return handler.wrapFunctionQuery(in, in.value(), q);
return q;
}
}

Expand All @@ -432,7 +442,7 @@ public static Query doTranslate(ScalarFunction f, TranslatorHandler handler) {
if (q != null) {
return q;
}
return handler.wrapFunctionQuery(f, f, new ScriptQuery(f.source(), f.asScript()));
return handler.wrapFunctionQuery(f, f, () -> new ScriptQuery(f.source(), f.asScript()));
}

public static Query doKnownTranslate(ScalarFunction f, TranslatorHandler handler) {
Expand Down
6 changes: 4 additions & 2 deletions x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/QlTranslatorHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import org.elasticsearch.xpack.ql.type.DataType;
import org.elasticsearch.xpack.ql.type.DataTypeConverter;

import java.util.function.Supplier;

public class QlTranslatorHandler implements TranslatorHandler {

@Override
Expand All @@ -24,9 +26,9 @@ public Query asQuery(Expression e) {
}

@Override
public Query wrapFunctionQuery(ScalarFunction sf, Expression field, Query q) {
public Query wrapFunctionQuery(ScalarFunction sf, Expression field, Supplier<Query> querySupplier) {
if (field instanceof FieldAttribute) {
return ExpressionTranslator.wrapIfNested(q, field);
return ExpressionTranslator.wrapIfNested(querySupplier.get(), field);
}
return new ScriptQuery(sf.source(), sf.asScript());
}
Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/planner/TranslatorHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import org.elasticsearch.xpack.ql.querydsl.query.Query;
import org.elasticsearch.xpack.ql.type.DataType;

import java.util.function.Supplier;

/**
* Parameterized handler used during query translation.
*
Expand All @@ -21,7 +23,7 @@ public interface TranslatorHandler {

Query asQuery(Expression e);

Query wrapFunctionQuery(ScalarFunction sf, Expression field, Query q);
Query wrapFunctionQuery(ScalarFunction sf, Expression field, Supplier<Query> querySupplier);

String nameOf(Expression e);

Expand Down
10 changes: 6 additions & 4 deletions ...ck/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/SqlTranslatorHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import org.elasticsearch.xpack.sql.expression.function.scalar.geo.StDistance;
import org.elasticsearch.xpack.sql.type.SqlDataTypeConverter;

import java.util.function.Supplier;

public class SqlTranslatorHandler implements TranslatorHandler {

private final boolean onAggs;
Expand All @@ -33,12 +35,12 @@ public Query asQuery(Expression e) {
}

@Override
public Query wrapFunctionQuery(ScalarFunction sf, Expression field, Query q) {
if (field instanceof StDistance && q instanceof GeoDistanceQuery) {
return ExpressionTranslator.wrapIfNested(q, ((StDistance) field).left());
public Query wrapFunctionQuery(ScalarFunction sf, Expression field, Supplier<Query> querySupplier) {
if (field instanceof StDistance && querySupplier.get() instanceof GeoDistanceQuery) {
return ExpressionTranslator.wrapIfNested(querySupplier.get(), ((StDistance) field).left());
}
if (field instanceof FieldAttribute) {
return ExpressionTranslator.wrapIfNested(q, field);
return ExpressionTranslator.wrapIfNested(querySupplier.get(), field);
}
return new ScriptQuery(sf.source(), sf.asScript());
}
Expand Down

0 comments on commit 6035c31

Please sign in to comment.