SQL: Fix issue with wrong NULL optimization

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Alias.java

@@ -76,7 +76,7 @@ public String qualifier() {
     }
 
     @Override
-    public boolean nullable() {
+    public Nullability nullable() {
         return child.nullable();
     }
 
@@ -121,4 +121,4 @@ private Attribute createAttribute() {
     public String toString() {
         return child + " AS " + name() + "#" + id();
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Attribute.java

@@ -7,8 +7,8 @@
 
 import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
 import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate;
-import org.elasticsearch.xpack.sql.tree.Source;
 import org.elasticsearch.xpack.sql.tree.NodeInfo;
+import org.elasticsearch.xpack.sql.tree.Source;
 
 import java.util.List;
 import java.util.Objects;
@@ -42,20 +42,20 @@ public abstract class Attribute extends NamedExpression {
     private final String qualifier;
 
     // can the attr be null - typically used in JOINs
-    private final boolean nullable;
+    private final Nullability nullability;
 
     public Attribute(Source source, String name, String qualifier, ExpressionId id) {
-        this(source, name, qualifier, true, id);
+        this(source, name, qualifier, Nullability.TRUE, id);
     }
 
-    public Attribute(Source source, String name, String qualifier, boolean nullable, ExpressionId id) {
-        this(source, name, qualifier, nullable, id, false);
+    public Attribute(Source source, String name, String qualifier, Nullability nullability, ExpressionId id) {
+        this(source, name, qualifier, nullability, id, false);
     }
 
-    public Attribute(Source source, String name, String qualifier, boolean nullable, ExpressionId id, boolean synthetic) {
+    public Attribute(Source source, String name, String qualifier, Nullability nullability, ExpressionId id, boolean synthetic) {
         super(source, name, emptyList(), id, synthetic);
         this.qualifier = qualifier;
-        this.nullable = nullable;
+        this.nullability = nullability;
     }
 
     @Override
@@ -77,8 +77,8 @@ public String qualifiedName() {
     }
 
     @Override
-    public boolean nullable() {
-        return nullable;
+    public Nullability nullable() {
+        return nullability;
     }
 
     @Override
@@ -94,11 +94,11 @@ public Attribute withQualifier(String qualifier) {
         return Objects.equals(qualifier(), qualifier) ? this : clone(source(), name(), qualifier, nullable(), id(), synthetic());
     }
 
-    public Attribute withNullability(boolean nullable) {
-        return Objects.equals(nullable(), nullable) ? this : clone(source(), name(), qualifier(), nullable, id(), synthetic());
+    public Attribute withNullability(Nullability nullability) {
+        return Objects.equals(nullable(), nullability) ? this : clone(source(), name(), qualifier(), nullability, id(), synthetic());
     }
 
-    protected abstract Attribute clone(Source source, String name, String qualifier, boolean nullable, ExpressionId id,
+    protected abstract Attribute clone(Source source, String name, String qualifier, Nullability nullability, ExpressionId id,
                                        boolean synthetic);
 
     @Override
@@ -123,15 +123,15 @@ public boolean semanticEquals(Expression other) {
 
     @Override
     public int hashCode() {
-        return Objects.hash(super.hashCode(), qualifier, nullable);
+        return Objects.hash(super.hashCode(), qualifier, nullability);
     }
 
     @Override
     public boolean equals(Object obj) {
         if (super.equals(obj)) {
             Attribute other = (Attribute) obj;
             return Objects.equals(qualifier, other.qualifier)
-                    && Objects.equals(nullable, other.nullable);
+                    && Objects.equals(nullability, other.nullability);
         }
 
         return false;
@@ -143,4 +143,4 @@ public String toString() {
     }
 
     protected abstract String label();
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Exists.java

@@ -36,7 +36,7 @@ public DataType dataType() {
     }
 
     @Override
-    public boolean nullable() {
-        return false;
+    public Nullability nullable() {
+        return Nullability.FALSE;
     }
 }

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expression.java

@@ -78,8 +78,7 @@ public Object fold() {
         throw new SqlIllegalArgumentException("Should not fold expression");
     }
 
-    // whether the expression becomes null if at least one param/input is null
-    public abstract boolean nullable();
+    public abstract Nullability nullable();
 
     // the references/inputs/leaves of the expression tree
     public AttributeSet references() {

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expressions.java

@@ -79,13 +79,8 @@ public static boolean match(List<? extends Expression> exps, Predicate<? super E
         return false;
     }
 
-    public static boolean nullable(List<? extends Expression> exps) {
-        for (Expression exp : exps) {
-            if (exp.nullable()) {
-                return true;
-            }
-        }
-        return false;
+    public static Nullability nullable(List<? extends Expression> exps) {
+       return Nullability.and(exps.stream().map(Expression::nullable).toArray(Nullability[]::new));
     }
 
     public static boolean foldable(List<? extends Expression> exps) {

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/FieldAttribute.java

@@ -6,8 +6,8 @@
 package org.elasticsearch.xpack.sql.expression;
 
 import org.elasticsearch.common.Strings;
-import org.elasticsearch.xpack.sql.tree.Source;
 import org.elasticsearch.xpack.sql.tree.NodeInfo;
+import org.elasticsearch.xpack.sql.tree.Source;
 import org.elasticsearch.xpack.sql.type.DataType;
 import org.elasticsearch.xpack.sql.type.EsField;
 import org.elasticsearch.xpack.sql.util.StringUtils;
@@ -34,12 +34,12 @@ public FieldAttribute(Source source, String name, EsField field) {
     }
 
     public FieldAttribute(Source source, FieldAttribute parent, String name, EsField field) {
-        this(source, parent, name, field, null, true, null, false);
+        this(source, parent, name, field, null, Nullability.TRUE, null, false);
     }
 
     public FieldAttribute(Source source, FieldAttribute parent, String name, EsField field, String qualifier,
-                          boolean nullable, ExpressionId id, boolean synthetic) {
-        super(source, name, field.getDataType(), qualifier, nullable, id, synthetic);
+                          Nullability nullability, ExpressionId id, boolean synthetic) {
+        super(source, name, field.getDataType(), qualifier, nullability, id, synthetic);
         this.path = parent != null ? parent.name() : StringUtils.EMPTY;
         this.parent = parent;
         this.field = field;
@@ -98,13 +98,14 @@ private FieldAttribute innerField(EsField type) {
 
     @Override
     protected Expression canonicalize() {
-        return new FieldAttribute(source(), null, "<none>", field, null, true, id(), false);
+        return new FieldAttribute(source(), null, "<none>", field, null, Nullability.TRUE, id(), false);
     }
 
     @Override
-    protected Attribute clone(Source source, String name, String qualifier, boolean nullable, ExpressionId id, boolean synthetic) {
+    protected Attribute clone(Source source, String name, String qualifier, Nullability nullability,
+                              ExpressionId id, boolean synthetic) {
         FieldAttribute qualifiedParent = parent != null ? (FieldAttribute) parent.withQualifier(qualifier) : null;
-        return new FieldAttribute(source, qualifiedParent, name, field, qualifier, nullable, id, synthetic);
+        return new FieldAttribute(source, qualifiedParent, name, field, qualifier, nullability, id, synthetic);
     }
 
     @Override

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Literal.java

@@ -8,8 +8,8 @@
 import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
 import org.elasticsearch.xpack.sql.expression.gen.script.Params;
 import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate;
-import org.elasticsearch.xpack.sql.tree.Source;
 import org.elasticsearch.xpack.sql.tree.NodeInfo;
+import org.elasticsearch.xpack.sql.tree.Source;
 import org.elasticsearch.xpack.sql.type.DataType;
 import org.elasticsearch.xpack.sql.type.DataTypeConversion;
 import org.elasticsearch.xpack.sql.type.DataTypes;
@@ -56,8 +56,8 @@ public boolean foldable() {
     }
 
     @Override
-    public boolean nullable() {
-        return value == null;
+    public Nullability nullable() {
+        return value == null ? Nullability.TRUE : Nullability.FALSE;
     }
 
     @Override
@@ -77,7 +77,7 @@ public Object fold() {
 
     @Override
     public Attribute toAttribute() {
-        return new LiteralAttribute(source(), name(), null, false, id(), false, dataType, this);
+        return new LiteralAttribute(source(), name(), null, Nullability.FALSE, id(), false, dataType, this);
     }
 
     @Override
@@ -168,4 +168,4 @@ public static Literal of(Expression source, Object value) {
         String name = source instanceof NamedExpression ? ((NamedExpression) source).name() : String.valueOf(value);
         return new Literal(source.source(), name, value, source.dataType());
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/LiteralAttribute.java

@@ -6,17 +6,17 @@
 package org.elasticsearch.xpack.sql.expression;
 
 import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe;
-import org.elasticsearch.xpack.sql.tree.Source;
 import org.elasticsearch.xpack.sql.tree.NodeInfo;
+import org.elasticsearch.xpack.sql.tree.Source;
 import org.elasticsearch.xpack.sql.type.DataType;
 
 public class LiteralAttribute extends TypedAttribute {
 
     private final Literal literal;
 
-    public LiteralAttribute(Source source, String name, String qualifier, boolean nullable, ExpressionId id, boolean synthetic,
-            DataType dataType, Literal literal) {
-        super(source, name, dataType, qualifier, nullable, id, synthetic);
+    public LiteralAttribute(Source source, String name, String qualifier, Nullability nullability, ExpressionId id, boolean synthetic,
+                            DataType dataType, Literal literal) {
+        super(source, name, dataType, qualifier, nullability, id, synthetic);
         this.literal = literal;
     }
 
@@ -27,9 +27,9 @@ protected NodeInfo<LiteralAttribute> info() {
     }
 
     @Override
-    protected LiteralAttribute clone(Source source, String name, String qualifier, boolean nullable,
+    protected LiteralAttribute clone(Source source, String name, String qualifier, Nullability nullability,
                                      ExpressionId id, boolean synthetic) {
-        return new LiteralAttribute(source, name, qualifier, nullable, id, synthetic, dataType(), literal);
+        return new LiteralAttribute(source, name, qualifier, nullability, id, synthetic, dataType(), literal);
     }
 
     @Override
@@ -41,4 +41,4 @@ protected String label() {
     public Pipe asPipe() {
         return literal.asPipe();
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Nullability.java

@@ -0,0 +1,38 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+package org.elasticsearch.xpack.sql.expression;
+
+public enum Nullability {
+    TRUE,    // Whether the expression can become null
+    FALSE,   // The expression can never become null
+    UNKNOWN; // Cannot determine if the expression supports possible null folding
+
+    /**
+     * Return the logical AND of a list of {@code Nullability}
+     * <pre>
+     *  UNKNOWN AND TRUE/FALSE/UNKNOWN = UNKNOWN
+     *  FALSE AND FALSE = FALSE
+     *  TRUE AND FALSE/TRUE = TRUE
+     * </pre>
+     */
+    public static Nullability and(Nullability... nullabilities) {
+        Nullability value = null;
+        for (Nullability n: nullabilities) {
+            switch (n) {
+                case UNKNOWN:
+                    return UNKNOWN;
+                case TRUE:
+                    value = TRUE;
+                    break;
+                case FALSE:
+                    if (value == null || value == FALSE) {
+                        value = FALSE;
+                    }
+            }
+        }
+        return value != null ? value : UNKNOWN;
+    }
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Order.java

@@ -41,8 +41,8 @@ protected NodeInfo<Order> info() {
     }
 
     @Override
-    public boolean nullable() {
-        return false;
+    public Nullability nullable() {
+        return Nullability.FALSE;
     }
 
     @Override
@@ -95,4 +95,4 @@ public boolean equals(Object obj) {
                 && Objects.equals(nulls, other.nulls)
                 && Objects.equals(child, other.child);
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/ScalarSubquery.java

@@ -36,7 +36,7 @@ public DataType dataType() {
     }
 
     @Override
-    public boolean nullable() {
-        return true;
+    public Nullability nullable() {
+        return Nullability.TRUE;
     }
 }

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/TypedAttribute.java

@@ -14,13 +14,9 @@ public abstract class TypedAttribute extends Attribute {
 
     private final DataType dataType;
 
-    protected TypedAttribute(Source source, String name, DataType dataType) {
-        this(source, name, dataType, null, true, null, false);
-    }
-
-    protected TypedAttribute(Source source, String name, DataType dataType, String qualifier, boolean nullable,
+    protected TypedAttribute(Source source, String name, DataType dataType, String qualifier, Nullability nullability,
             ExpressionId id, boolean synthetic) {
-        super(source, name, qualifier, nullable, id, synthetic);
+        super(source, name, qualifier, nullability, id, synthetic);
         this.dataType = dataType;
     }
 

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/UnaryExpression.java

@@ -41,7 +41,7 @@ public boolean foldable() {
     }
 
     @Override
-    public boolean nullable() {
+    public Nullability nullable() {
         return child.nullable();
     }
 
@@ -73,4 +73,4 @@ public boolean equals(Object obj) {
         UnaryExpression other = (UnaryExpression) obj;
         return Objects.equals(child, other.child);
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/UnresolvedAlias.java

@@ -46,7 +46,7 @@ public String unresolvedMessage() {
     }
 
     @Override
-    public boolean nullable() {
+    public Nullability nullable() {
         throw new UnresolvedException("nullable", this);
     }
 

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/UnresolvedAttribute.java

@@ -7,8 +7,8 @@
 
 import org.elasticsearch.xpack.sql.capabilities.Unresolvable;
 import org.elasticsearch.xpack.sql.capabilities.UnresolvedException;
-import org.elasticsearch.xpack.sql.tree.Source;
 import org.elasticsearch.xpack.sql.tree.NodeInfo;
+import org.elasticsearch.xpack.sql.tree.Source;
 import org.elasticsearch.xpack.sql.type.DataType;
 import org.elasticsearch.xpack.sql.util.CollectionUtils;
 
@@ -65,7 +65,8 @@ public boolean resolved() {
     }
 
     @Override
-    protected Attribute clone(Source source, String name, String qualifier, boolean nullable, ExpressionId id, boolean synthetic) {
+    protected Attribute clone(Source source, String name, String qualifier, Nullability nullability,
+                              ExpressionId id, boolean synthetic) {
         return this;
     }
 

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/UnresolvedStar.java

@@ -35,7 +35,7 @@ public Expression replaceChildren(List<Expression> newChildren) {
     }
 
     @Override
-    public boolean nullable() {
+    public Nullability nullable() {
         throw new UnresolvedException("nullable", this);
     }