Skip to content

Commit

Permalink
SQL: Handle null literal for AND and OR in WHERE (#35236)
Browse files Browse the repository at this point in the history
Change `nullable()` logic of AND and OR to false since in the Optimizer
we cannot fold to null as we might be handling and expression in the
SELECT clause.

Introduce folding of null for AND and OR expressions in PruneFilter()
since we now know that we are in HAVING or WHERE clause and we
can fold `null` to `false`

Fixes: #35088

Co-authored-by: Costin Leau <costin.leau@gmail.com>
  • Loading branch information
2 people authored and matriv committed Nov 8, 2018
1 parent fe988a2 commit 4d0aba2
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 11 deletions.
Expand Up @@ -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;
}
}
Expand Up @@ -683,17 +683,46 @@ static class PruneFilters extends OptimizerRule<Filter> {

@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<Filter> {
Expand Down Expand Up @@ -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;
}
}
Expand Down
Expand Up @@ -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());
Expand Down

0 comments on commit 4d0aba2

Please sign in to comment.