diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/Neg.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/Neg.java index ebd44a44abc..c297604fb23 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/Neg.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/Neg.java @@ -11,7 +11,6 @@ import org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal; import org.elasticsearch.xpack.sql.expression.NamedExpression; import org.elasticsearch.xpack.sql.expression.function.scalar.UnaryScalarFunction; import org.elasticsearch.xpack.sql.expression.gen.processor.Processor; -import org.elasticsearch.xpack.sql.expression.gen.script.ScriptWeaver; import org.elasticsearch.xpack.sql.expression.gen.script.Scripts; import org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic.UnaryArithmeticProcessor.UnaryArithmeticOperation; import org.elasticsearch.xpack.sql.tree.Location; @@ -21,7 +20,7 @@ import org.elasticsearch.xpack.sql.type.DataType; /** * Negation function (@{code -x}). */ -public class Neg extends UnaryScalarFunction implements ScriptWeaver { +public class Neg extends UnaryScalarFunction { public Neg(Location location, Expression field) { super(location, field); diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/In.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/In.java index 42c28016e4b..329dc307da8 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/In.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/In.java @@ -5,15 +5,12 @@ */ package org.elasticsearch.xpack.sql.expression.predicate.operator.comparison; -import org.elasticsearch.xpack.sql.expression.Attribute; import org.elasticsearch.xpack.sql.expression.Expression; import org.elasticsearch.xpack.sql.expression.Expressions; import org.elasticsearch.xpack.sql.expression.Foldables; -import org.elasticsearch.xpack.sql.expression.NamedExpression; -import org.elasticsearch.xpack.sql.expression.function.scalar.ScalarFunctionAttribute; +import org.elasticsearch.xpack.sql.expression.function.scalar.ScalarFunction; import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe; import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate; -import org.elasticsearch.xpack.sql.expression.gen.script.ScriptWeaver; import org.elasticsearch.xpack.sql.tree.Location; import org.elasticsearch.xpack.sql.tree.NodeInfo; import org.elasticsearch.xpack.sql.type.DataType; @@ -29,14 +26,13 @@ import java.util.stream.Collectors; import static org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder.paramsBuilder; -public class In extends NamedExpression implements ScriptWeaver { +public class In extends ScalarFunction { private final Expression value; private final List list; - private Attribute lazyAttribute; public In(Location location, Expression value, List list) { - super(location, null, CollectionUtils.combine(list, value), null); + super(location, CollectionUtils.combine(list, value)); this.value = value; this.list = new ArrayList<>(new LinkedHashSet<>(list)); } @@ -95,15 +91,6 @@ public class In extends NamedExpression implements ScriptWeaver { return Expressions.name(value) + sj.toString(); } - @Override - public Attribute toAttribute() { - if (lazyAttribute == null) { - lazyAttribute = new ScalarFunctionAttribute(location(), name(), dataType(), null, - false, id(), false, "IN", asScript(), null, asPipe()); - } - return lazyAttribute; - } - @Override public ScriptTemplate asScript() { ScriptTemplate leftScript = asScript(value); 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 f58d290d0e8..2713a01816c 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 @@ -30,8 +30,6 @@ import org.elasticsearch.xpack.sql.expression.function.aggregate.Sum; import org.elasticsearch.xpack.sql.expression.function.scalar.ScalarFunction; import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeFunction; import org.elasticsearch.xpack.sql.expression.function.scalar.datetime.DateTimeHistogramFunction; -import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate; -import org.elasticsearch.xpack.sql.expression.predicate.nulls.IsNull; import org.elasticsearch.xpack.sql.expression.predicate.Range; import org.elasticsearch.xpack.sql.expression.predicate.fulltext.MatchQueryPredicate; import org.elasticsearch.xpack.sql.expression.predicate.fulltext.MultiMatchQueryPredicate; @@ -40,6 +38,7 @@ import org.elasticsearch.xpack.sql.expression.predicate.logical.And; import org.elasticsearch.xpack.sql.expression.predicate.logical.Not; import org.elasticsearch.xpack.sql.expression.predicate.logical.Or; import org.elasticsearch.xpack.sql.expression.predicate.nulls.IsNotNull; +import org.elasticsearch.xpack.sql.expression.predicate.nulls.IsNull; import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.BinaryComparison; import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.Equals; import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.GreaterThan; @@ -94,6 +93,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Optional; +import java.util.function.Supplier; import static java.util.Collections.singletonList; import static org.elasticsearch.xpack.sql.expression.Foldables.doubleValuesOf; @@ -487,11 +487,8 @@ final class QueryTranslator { if (onAggs) { aggFilter = new AggFilter(not.id().toString(), not.asScript()); } else { - query = new NotQuery(not.location(), toQuery(not.field(), false).query); - // query directly on the field - if (not.field() instanceof FieldAttribute) { - query = wrapIfNested(query, not.field()); - } + query = handleQuery(not, not.field(), + () -> new NotQuery(not.location(), toQuery(not.field(), false).query)); } return new QueryTranslation(query, aggFilter); @@ -508,11 +505,8 @@ final class QueryTranslator { if (onAggs) { aggFilter = new AggFilter(isNotNull.id().toString(), isNotNull.asScript()); } else { - query = new ExistsQuery(isNotNull.location(), nameOf(isNotNull.field())); - // query directly on the field - if (isNotNull.field() instanceof NamedExpression) { - query = wrapIfNested(query, isNotNull.field()); - } + query = handleQuery(isNotNull, isNotNull.field(), + () -> new ExistsQuery(isNotNull.location(), nameOf(isNotNull.field()))); } return new QueryTranslation(query, aggFilter); @@ -529,11 +523,8 @@ final class QueryTranslator { if (onAggs) { aggFilter = new AggFilter(isNull.id().toString(), isNull.asScript()); } else { - query = new NotQuery(isNull.location(), new ExistsQuery(isNull.location(), nameOf(isNull.field()))); - // query directly on the field - if (isNull.field() instanceof NamedExpression) { - query = wrapIfNested(query, isNull.field()); - } + query = handleQuery(isNull, isNull.field(), + () -> new NotQuery(isNull.location(), new ExistsQuery(isNull.location(), nameOf(isNull.field())))); } return new QueryTranslation(query, aggFilter); @@ -564,12 +555,7 @@ final class QueryTranslator { aggFilter = new AggFilter(at.id().toString(), bc.asScript()); } else { - // query directly on the field - if (at instanceof FieldAttribute) { - query = wrapIfNested(translateQuery(bc), ne); - } else { - query = new ScriptQuery(at.location(), bc.asScript()); - } + query = handleQuery(bc, ne, () -> translateQuery(bc)); } return new QueryTranslation(query, aggFilter); } @@ -646,17 +632,11 @@ final class QueryTranslator { // // Agg context means HAVING -> PipelineAggs // - ScriptTemplate script = in.asScript(); if (onAggs) { - aggFilter = new AggFilter(at.id().toString(), script); + aggFilter = new AggFilter(at.id().toString(), in.asScript()); } else { - // query directly on the field - if (at instanceof FieldAttribute) { - query = wrapIfNested(new TermsQuery(in.location(), ne.name(), in.list()), ne); - } else { - query = new ScriptQuery(at.location(), script); - } + query = handleQuery(in, ne, () -> new TermsQuery(in.location(), ne.name(), in.list())); } return new QueryTranslation(query, aggFilter); } @@ -687,16 +667,9 @@ final class QueryTranslator { if (onAggs) { aggFilter = new AggFilter(at.id().toString(), r.asScript()); } else { - // typical range; no scripting involved - if (at instanceof FieldAttribute) { - RangeQuery rangeQuery = new RangeQuery(r.location(), nameOf(r.value()), valueOf(r.lower()), r.includeLower(), - valueOf(r.upper()), r.includeUpper(), dateFormat(r.value())); - query = wrapIfNested(rangeQuery, r.value()); - } - // scripted query - else { - query = new ScriptQuery(at.location(), r.asScript()); - } + query = handleQuery(r, r.value(), + () -> new RangeQuery(r.location(), nameOf(r.value()), valueOf(r.lower()), r.includeLower(), + valueOf(r.upper()), r.includeUpper(), dateFormat(r.value()))); } return new QueryTranslation(query, aggFilter); } else { @@ -845,6 +818,14 @@ final class QueryTranslator { protected abstract QueryTranslation asQuery(E e, boolean onAggs); + + protected static Query handleQuery(ScalarFunction sf, Expression field, Supplier query) { + if (field instanceof FieldAttribute) { + return wrapIfNested(query.get(), field); + } + return new ScriptQuery(sf.location(), sf.asScript()); + } + protected static Query wrapIfNested(Query query, Expression exp) { if (exp instanceof FieldAttribute) { FieldAttribute fa = (FieldAttribute) exp; 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 92f953cb147..e2c42874696 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 @@ -163,6 +163,22 @@ public class QueryTranslatorTests extends ESTestCase { assertEquals("Scalar function (LTRIM(keyword)) not allowed (yet) as arguments for LIKE", ex.getMessage()); } + public void testTranslateNotExpression_WhereClause_Painless() { + LogicalPlan p = plan("SELECT * FROM test WHERE NOT(POSITION('x', keyword) = 0)"); + assertTrue(p instanceof Project); + assertTrue(p.children().get(0) instanceof Filter); + Expression condition = ((Filter) p.children().get(0)).condition(); + assertFalse(condition.foldable()); + QueryTranslation translation = QueryTranslator.toQuery(condition, false); + assertTrue(translation.query instanceof ScriptQuery); + ScriptQuery sc = (ScriptQuery) translation.query; + assertEquals("InternalSqlScriptUtils.nullSafeFilter(InternalSqlScriptUtils.not(" + + "InternalSqlScriptUtils.eq(InternalSqlScriptUtils.position(" + + "params.v0,InternalSqlScriptUtils.docValue(doc,params.v1)),params.v2)))", + sc.script().toString()); + assertEquals("[{v=x}, {v=keyword}, {v=0}]", sc.script().params().toString()); + } + public void testTranslateIsNullExpression_WhereClause() { LogicalPlan p = plan("SELECT * FROM test WHERE keyword IS NULL"); assertTrue(p instanceof Project); @@ -178,6 +194,21 @@ public class QueryTranslatorTests extends ESTestCase { eq.asBuilder().toString().replaceAll("\\s+", "")); } + public void testTranslateIsNullExpression_WhereClause_Painless() { + LogicalPlan p = plan("SELECT * FROM test WHERE POSITION('x', keyword) IS NULL"); + assertTrue(p instanceof Project); + assertTrue(p.children().get(0) instanceof Filter); + Expression condition = ((Filter) p.children().get(0)).condition(); + assertFalse(condition.foldable()); + QueryTranslation translation = QueryTranslator.toQuery(condition, false); + assertTrue(translation.query instanceof ScriptQuery); + ScriptQuery sc = (ScriptQuery) translation.query; + assertEquals("InternalSqlScriptUtils.nullSafeFilter(InternalSqlScriptUtils.isNull(" + + "InternalSqlScriptUtils.position(params.v0,InternalSqlScriptUtils.docValue(doc,params.v1))))", + sc.script().toString()); + assertEquals("[{v=x}, {v=keyword}]", sc.script().params().toString()); + } + public void testTranslateIsNotNullExpression_WhereClause() { LogicalPlan p = plan("SELECT * FROM test WHERE keyword IS NOT NULL"); assertTrue(p instanceof Project); @@ -191,6 +222,21 @@ public class QueryTranslatorTests extends ESTestCase { eq.asBuilder().toString().replaceAll("\\s+", "")); } + public void testTranslateIsNotNullExpression_WhereClause_Painless() { + LogicalPlan p = plan("SELECT * FROM test WHERE POSITION('x', keyword) IS NOT NULL"); + assertTrue(p instanceof Project); + assertTrue(p.children().get(0) instanceof Filter); + Expression condition = ((Filter) p.children().get(0)).condition(); + assertFalse(condition.foldable()); + QueryTranslation translation = QueryTranslator.toQuery(condition, false); + assertTrue(translation.query instanceof ScriptQuery); + ScriptQuery sc = (ScriptQuery) translation.query; + assertEquals("InternalSqlScriptUtils.nullSafeFilter(InternalSqlScriptUtils.isNotNull(" + + "InternalSqlScriptUtils.position(params.v0,InternalSqlScriptUtils.docValue(doc,params.v1))))", + sc.script().toString()); + assertEquals("[{v=x}, {v=keyword}]", sc.script().params().toString()); + } + public void testTranslateIsNullExpression_HavingClause_Painless() { LogicalPlan p = plan("SELECT keyword, max(int) FROM test GROUP BY keyword HAVING max(int) IS NULL"); assertTrue(p instanceof Project);