diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/logical/BinaryLogic.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/logical/BinaryLogic.java index 93c50fbc1351b..5e4175797eb58 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/logical/BinaryLogic.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/logical/BinaryLogic.java @@ -36,6 +36,7 @@ protected Pipe makePipe() { @Override public boolean nullable() { - return left().nullable() && right().nullable(); + // Cannot fold null due to 3vl, constant folding will do any possible folding. + return false; } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java index ca69a53ca86ef..e9fc6b06c89b3 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java @@ -683,17 +683,46 @@ static class PruneFilters extends OptimizerRule { @Override protected LogicalPlan rule(Filter filter) { - if (filter.condition() instanceof Literal) { - if (TRUE.equals(filter.condition())) { + Expression condition = filter.condition().transformUp(PruneFilters::foldBinaryLogic); + + if (condition instanceof Literal) { + if (TRUE.equals(condition)) { return filter.child(); } - if (FALSE.equals(filter.condition()) || FoldNull.isNull(filter.condition())) { + if (FALSE.equals(condition) || FoldNull.isNull(condition)) { return new LocalRelation(filter.location(), new EmptyExecutable(filter.output())); } } + if (!condition.equals(filter.condition())) { + return new Filter(filter.location(), filter.child(), condition); + } return filter; } + + private static Expression foldBinaryLogic(Expression expression) { + if (expression instanceof Or) { + Or or = (Or) expression; + boolean nullLeft = FoldNull.isNull(or.left()); + boolean nullRight = FoldNull.isNull(or.right()); + if (nullLeft && nullRight) { + return Literal.NULL; + } + if (nullLeft) { + return or.right(); + } + if (nullRight) { + return or.left(); + } + } + if (expression instanceof And) { + And and = (And) expression; + if (FoldNull.isNull(and.left()) || FoldNull.isNull(and.right())) { + return Literal.NULL; + } + } + return expression; + } } static class ReplaceAliasesInHaving extends OptimizerRule { @@ -1132,21 +1161,18 @@ protected Expression rule(Expression e) { if (((IsNotNull) e).field().nullable() == false) { return new Literal(e.location(), Expressions.name(e), Boolean.TRUE, DataType.BOOLEAN); } - } - // see https://github.com/elastic/elasticsearch/issues/34876 - // similar for IsNull once it gets introduced + // see https://github.com/elastic/elasticsearch/issues/34876 + // similar for IsNull once it gets introduced - if (e instanceof In) { + } else if (e instanceof In) { In in = (In) e; if (isNull(in.value())) { return Literal.of(in, null); } - } - if (e.nullable() && Expressions.anyMatch(e.children(), FoldNull::isNull)) { + } else if (e.nullable() && Expressions.anyMatch(e.children(), FoldNull::isNull)) { return Literal.of(e, null); } - return e; } } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java index 77606ab1390dd..0ca0df6fc08ff 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryFolderTests.java @@ -65,6 +65,52 @@ public void testFoldingToLocalExecWithProject() { assertThat(ee.output().get(0).toString(), startsWith("keyword{f}#")); } + public void testFoldingToLocalExecBooleanAndNull_WhereClause2() { + PhysicalPlan p = plan("SELECT true OR null"); + } + public void testFoldingToLocalExecBooleanAndNull_WhereClause() { + PhysicalPlan p = plan("SELECT keyword FROM test WHERE int > 10 AND null AND true"); + assertEquals(LocalExec.class, p.getClass()); + LocalExec le = (LocalExec) p; + assertEquals(EmptyExecutable.class, le.executable().getClass()); + EmptyExecutable ee = (EmptyExecutable) le.executable(); + assertEquals(1, ee.output().size()); + assertThat(ee.output().get(0).toString(), startsWith("keyword{f}#")); + } + + public void testFoldingToLocalExecBooleanAndNull_HavingClause() { + PhysicalPlan p = plan("SELECT keyword, max(int) FROM test GROUP BY keyword HAVING max(int) > 10 AND null"); + assertEquals(LocalExec.class, p.getClass()); + LocalExec le = (LocalExec) p; + assertEquals(EmptyExecutable.class, le.executable().getClass()); + EmptyExecutable ee = (EmptyExecutable) le.executable(); + assertEquals(2, ee.output().size()); + assertThat(ee.output().get(0).toString(), startsWith("keyword{f}#")); + assertThat(ee.output().get(1).toString(), startsWith("MAX(int){a->")); + } + + public void testFoldingBooleanOrNull_WhereClause() { + PhysicalPlan p = plan("SELECT keyword FROM test WHERE int > 10 OR null OR false"); + assertEquals(EsQueryExec.class, p.getClass()); + EsQueryExec ee = (EsQueryExec) p; + assertEquals("{\"range\":{\"int\":{\"from\":10,\"to\":null,\"include_lower\":false,\"include_upper\":false,\"boost\":1.0}}}", + ee.queryContainer().query().asBuilder().toString().replaceAll("\\s+", "")); + assertEquals(1, ee.output().size()); + assertThat(ee.output().get(0).toString(), startsWith("keyword{f}#")); + } + + public void testFoldingBooleanOrNull_HavingClause() { + PhysicalPlan p = plan("SELECT keyword, max(int) FROM test GROUP BY keyword HAVING max(int) > 10 OR null"); + assertEquals(EsQueryExec.class, p.getClass()); + EsQueryExec ee = (EsQueryExec) p; + assertTrue(ee.queryContainer().aggs().asAggBuilder().toString().replaceAll("\\s+", "").contains( + "\"script\":{\"source\":\"InternalSqlScriptUtils.nullSafeFilter(InternalSqlScriptUtils.gt(params.a0,params.v0))\"," + + "\"lang\":\"painless\",\"params\":{\"v0\":10}},")); + assertEquals(2, ee.output().size()); + assertThat(ee.output().get(0).toString(), startsWith("keyword{f}#")); + assertThat(ee.output().get(1).toString(), startsWith("MAX(int){a->")); + } + public void testFoldingOfIsNotNull() { PhysicalPlan p = plan("SELECT keyword FROM test WHERE (keyword IS NULL) IS NOT NULL"); assertEquals(EsQueryExec.class, p.getClass());