From da3d8fb5b74e99b553e4ab4e04ad8b3fd294e4b3 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Sun, 6 Jan 2019 18:29:34 +0200 Subject: [PATCH] SQL: Fix issue with wrong NULL optimization (#37124) Logical operators OR and AND as well as conditional functions (COALESCE, LEAST, GREATEST, etc.) cannot be folded to NULL if one of their children is NULL as is the case for most of the functions. Therefore, their nullable() implementation cannot return true. On the other hand they cannot return false as if they're wrapped within an IS NULL or IS NOT NULL expression, the expression will be folded to false and true respectively leading to wrong results. Change the signature of nullable() method and add a third value UKNOWN to handle these cases. Fixes: #35872 --- .../xpack/sql/expression/Alias.java | 4 +- .../xpack/sql/expression/Attribute.java | 30 +++++++-------- .../xpack/sql/expression/Exists.java | 4 +- .../xpack/sql/expression/Expression.java | 3 +- .../xpack/sql/expression/Expressions.java | 9 +---- .../xpack/sql/expression/FieldAttribute.java | 15 ++++---- .../xpack/sql/expression/Literal.java | 10 ++--- .../sql/expression/LiteralAttribute.java | 14 +++---- .../xpack/sql/expression/Nullability.java | 38 +++++++++++++++++++ .../xpack/sql/expression/Order.java | 6 +-- .../xpack/sql/expression/ScalarSubquery.java | 4 +- .../xpack/sql/expression/TypedAttribute.java | 8 +--- .../xpack/sql/expression/UnaryExpression.java | 4 +- .../xpack/sql/expression/UnresolvedAlias.java | 2 +- .../sql/expression/UnresolvedAttribute.java | 5 ++- .../xpack/sql/expression/UnresolvedStar.java | 2 +- .../sql/expression/function/Function.java | 3 +- .../function/FunctionAttribute.java | 7 ++-- .../expression/function/ScoreAttribute.java | 16 +++++--- .../function/UnresolvedFunction.java | 3 +- .../aggregate/AggregateFunctionAttribute.java | 15 ++++---- .../grouping/GroupingFunctionAttribute.java | 16 ++++---- .../sql/expression/function/scalar/Cast.java | 8 +++- .../scalar/ConfigurationFunction.java | 7 ++-- .../scalar/ScalarFunctionAttribute.java | 22 ++++++----- .../function/scalar/string/Concat.java | 7 ++-- .../xpack/sql/expression/predicate/Range.java | 7 ++-- .../conditional/ConditionalFunction.java | 5 ++- .../predicate/conditional/NullIf.java | 5 ++- .../predicate/fulltext/FullTextPredicate.java | 7 ++-- .../predicate/logical/BinaryLogic.java | 5 ++- .../expression/predicate/nulls/IsNotNull.java | 5 ++- .../expression/predicate/nulls/IsNull.java | 5 ++- .../predicate/operator/comparison/In.java | 5 ++- .../operator/comparison/NullEquals.java | 5 ++- .../predicate/regex/RegexMatch.java | 8 +++- .../xpack/sql/optimizer/Optimizer.java | 15 +++----- .../xpack/sql/plan/logical/Join.java | 16 ++++---- .../sql/expression/NullabilityTests.java | 35 +++++++++++++++++ .../xpack/sql/optimizer/OptimizerTests.java | 35 ++++++++++++++++- 40 files changed, 272 insertions(+), 148 deletions(-) create mode 100644 x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Nullability.java create mode 100644 x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/NullabilityTests.java diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Alias.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Alias.java index 1a2e4759a92..576bd233ac6 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Alias.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Alias.java @@ -76,7 +76,7 @@ public class Alias extends NamedExpression { } @Override - public boolean nullable() { + public Nullability nullable() { return child.nullable(); } @@ -121,4 +121,4 @@ public class Alias extends NamedExpression { public String toString() { return child + " AS " + name() + "#" + id(); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Attribute.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Attribute.java index fa4ec60fc36..2f8b6633249 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Attribute.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Attribute.java @@ -7,8 +7,8 @@ package org.elasticsearch.xpack.sql.expression; 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 abstract class Attribute extends NamedExpression { } @Override - public boolean nullable() { - return nullable; + public Nullability nullable() { + return nullability; } @Override @@ -94,11 +94,11 @@ public abstract class Attribute extends NamedExpression { 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,7 +123,7 @@ public abstract class Attribute extends NamedExpression { @Override public int hashCode() { - return Objects.hash(super.hashCode(), qualifier, nullable); + return Objects.hash(super.hashCode(), qualifier, nullability); } @Override @@ -131,7 +131,7 @@ public abstract class Attribute extends NamedExpression { 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 abstract class Attribute extends NamedExpression { } protected abstract String label(); -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Exists.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Exists.java index 7990b457171..2363b52316c 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Exists.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Exists.java @@ -36,7 +36,7 @@ public class Exists extends SubQueryExpression { } @Override - public boolean nullable() { - return false; + public Nullability nullable() { + return Nullability.FALSE; } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expression.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expression.java index 8cd438d3171..d421d2b01c0 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expression.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expression.java @@ -78,8 +78,7 @@ public abstract class Expression extends Node implements Resolvable 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() { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expressions.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expressions.java index 77b9310fcd1..967ce6cc6a6 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expressions.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Expressions.java @@ -79,13 +79,8 @@ public final class Expressions { return false; } - public static boolean nullable(List exps) { - for (Expression exp : exps) { - if (exp.nullable()) { - return true; - } - } - return false; + public static Nullability nullable(List exps) { + return Nullability.and(exps.stream().map(Expression::nullable).toArray(Nullability[]::new)); } public static boolean foldable(List exps) { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/FieldAttribute.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/FieldAttribute.java index fbd79e2fe2c..832af029df3 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/FieldAttribute.java +++ b/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 class FieldAttribute extends TypedAttribute { } 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 @@ public class FieldAttribute extends TypedAttribute { @Override protected Expression canonicalize() { - return new FieldAttribute(source(), null, "", field, null, true, id(), false); + return new FieldAttribute(source(), null, "", 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 diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Literal.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Literal.java index 33dbeb2f769..1d029f31330 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Literal.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Literal.java @@ -8,8 +8,8 @@ package org.elasticsearch.xpack.sql.expression; 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 class Literal extends NamedExpression { } @Override - public boolean nullable() { - return value == null; + public Nullability nullable() { + return value == null ? Nullability.TRUE : Nullability.FALSE; } @Override @@ -77,7 +77,7 @@ public class Literal extends NamedExpression { @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 class Literal extends NamedExpression { String name = source instanceof NamedExpression ? ((NamedExpression) source).name() : String.valueOf(value); return new Literal(source.source(), name, value, source.dataType()); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/LiteralAttribute.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/LiteralAttribute.java index 2fdc13fa89c..6463520cf83 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/LiteralAttribute.java +++ b/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 @@ public class LiteralAttribute extends TypedAttribute { } @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 @@ public class LiteralAttribute extends TypedAttribute { public Pipe asPipe() { return literal.asPipe(); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Nullability.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Nullability.java new file mode 100644 index 00000000000..ac46bdf2aa9 --- /dev/null +++ b/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} + *
+     *  UNKNOWN AND TRUE/FALSE/UNKNOWN = UNKNOWN
+     *  FALSE AND FALSE = FALSE
+     *  TRUE AND FALSE/TRUE = TRUE
+     * 
+ */ + 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; + } + } + } + return value != null ? value : FALSE; + } +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Order.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Order.java index ff710772f09..6a57c3275d4 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Order.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/Order.java @@ -41,8 +41,8 @@ public class Order extends Expression { } @Override - public boolean nullable() { - return false; + public Nullability nullable() { + return Nullability.FALSE; } @Override @@ -95,4 +95,4 @@ public class Order extends Expression { && Objects.equals(nulls, other.nulls) && Objects.equals(child, other.child); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/ScalarSubquery.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/ScalarSubquery.java index feb103baba7..84693cdc79d 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/ScalarSubquery.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/ScalarSubquery.java @@ -36,7 +36,7 @@ public class ScalarSubquery extends SubQueryExpression { } @Override - public boolean nullable() { - return true; + public Nullability nullable() { + return Nullability.TRUE; } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/TypedAttribute.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/TypedAttribute.java index c04ccf51ce0..414ff330bda 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/TypedAttribute.java +++ b/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; } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/UnaryExpression.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/UnaryExpression.java index 186065d60f8..69cb5107042 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/UnaryExpression.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/UnaryExpression.java @@ -41,7 +41,7 @@ public abstract class UnaryExpression extends Expression { } @Override - public boolean nullable() { + public Nullability nullable() { return child.nullable(); } @@ -73,4 +73,4 @@ public abstract class UnaryExpression extends Expression { UnaryExpression other = (UnaryExpression) obj; return Objects.equals(child, other.child); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/UnresolvedAlias.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/UnresolvedAlias.java index e0398be24b1..178c4d89695 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/UnresolvedAlias.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/UnresolvedAlias.java @@ -46,7 +46,7 @@ public class UnresolvedAlias extends UnresolvedNamedExpression { } @Override - public boolean nullable() { + public Nullability nullable() { throw new UnresolvedException("nullable", this); } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/UnresolvedAttribute.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/UnresolvedAttribute.java index b1536fa8464..476c69fea09 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/UnresolvedAttribute.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/UnresolvedAttribute.java @@ -7,8 +7,8 @@ package org.elasticsearch.xpack.sql.expression; 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 class UnresolvedAttribute extends Attribute implements Unresolvable { } @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; } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/UnresolvedStar.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/UnresolvedStar.java index 3dba3a8950e..4b5a6dfa537 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/UnresolvedStar.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/UnresolvedStar.java @@ -35,7 +35,7 @@ public class UnresolvedStar extends UnresolvedNamedExpression { } @Override - public boolean nullable() { + public Nullability nullable() { throw new UnresolvedException("nullable", this); } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/Function.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/Function.java index 0071e517ac0..f63145f6a25 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/Function.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/Function.java @@ -9,6 +9,7 @@ import org.elasticsearch.xpack.sql.expression.Expression; import org.elasticsearch.xpack.sql.expression.ExpressionId; import org.elasticsearch.xpack.sql.expression.Expressions; import org.elasticsearch.xpack.sql.expression.NamedExpression; +import org.elasticsearch.xpack.sql.expression.Nullability; import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.util.StringUtils; @@ -45,7 +46,7 @@ public abstract class Function extends NamedExpression { } @Override - public boolean nullable() { + public Nullability nullable() { return Expressions.nullable(children()); } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionAttribute.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionAttribute.java index 4c9e3f5a2cd..36ff097bdae 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionAttribute.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/FunctionAttribute.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.sql.expression.function; import org.elasticsearch.xpack.sql.expression.ExpressionId; +import org.elasticsearch.xpack.sql.expression.Nullability; import org.elasticsearch.xpack.sql.expression.TypedAttribute; import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.type.DataType; @@ -16,9 +17,9 @@ public abstract class FunctionAttribute extends TypedAttribute { private final String functionId; - protected FunctionAttribute(Source source, String name, DataType dataType, String qualifier, boolean nullable, ExpressionId id, - boolean synthetic, String functionId) { - super(source, name, dataType, qualifier, nullable, id, synthetic); + protected FunctionAttribute(Source source, String name, DataType dataType, String qualifier, Nullability nullability, + ExpressionId id, boolean synthetic, String functionId) { + super(source, name, dataType, qualifier, nullability, id, synthetic); this.functionId = functionId; } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/ScoreAttribute.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/ScoreAttribute.java index 43925b27cf9..bcd0aab16c6 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/ScoreAttribute.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/ScoreAttribute.java @@ -7,12 +7,15 @@ package org.elasticsearch.xpack.sql.expression.function; import org.elasticsearch.xpack.sql.expression.Attribute; import org.elasticsearch.xpack.sql.expression.ExpressionId; +import org.elasticsearch.xpack.sql.expression.Nullability; import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe; import org.elasticsearch.xpack.sql.expression.gen.pipeline.ScorePipe; -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 static org.elasticsearch.xpack.sql.expression.Nullability.FALSE; + /** * {@link Attribute} that represents Elasticsearch's {@code _score}. */ @@ -21,15 +24,15 @@ public class ScoreAttribute extends FunctionAttribute { * Constructor for normal use. */ public ScoreAttribute(Source source) { - this(source, "SCORE()", DataType.FLOAT, null, false, null, false); + this(source, "SCORE()", DataType.FLOAT, null, FALSE, null, false); } /** * Constructor for {@link #clone()} */ - private ScoreAttribute(Source source, String name, DataType dataType, String qualifier, boolean nullable, ExpressionId id, + private ScoreAttribute(Source source, String name, DataType dataType, String qualifier, Nullability nullability, ExpressionId id, boolean synthetic) { - super(source, name, dataType, qualifier, nullable, id, synthetic, "SCORE"); + super(source, name, dataType, qualifier, nullability, id, synthetic, "SCORE"); } @Override @@ -38,8 +41,9 @@ public class ScoreAttribute extends FunctionAttribute { } @Override - protected Attribute clone(Source source, String name, String qualifier, boolean nullable, ExpressionId id, boolean synthetic) { - return new ScoreAttribute(source, name, dataType(), qualifier, nullable, id, synthetic); + protected Attribute clone(Source source, String name, String qualifier, Nullability nullability, + ExpressionId id, boolean synthetic) { + return new ScoreAttribute(source, name, dataType(), qualifier, nullability, id, synthetic); } @Override diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/UnresolvedFunction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/UnresolvedFunction.java index cacf1045487..13e038977ae 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/UnresolvedFunction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/UnresolvedFunction.java @@ -10,6 +10,7 @@ import org.elasticsearch.xpack.sql.capabilities.UnresolvedException; import org.elasticsearch.xpack.sql.expression.Attribute; import org.elasticsearch.xpack.sql.expression.Expression; import org.elasticsearch.xpack.sql.expression.Literal; +import org.elasticsearch.xpack.sql.expression.Nullability; import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate; import org.elasticsearch.xpack.sql.session.Configuration; import org.elasticsearch.xpack.sql.tree.Source; @@ -135,7 +136,7 @@ public class UnresolvedFunction extends Function implements Unresolvable { } @Override - public boolean nullable() { + public Nullability nullable() { throw new UnresolvedException("nullable", this); } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/AggregateFunctionAttribute.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/AggregateFunctionAttribute.java index 22456e10bdd..b50b268844c 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/AggregateFunctionAttribute.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/AggregateFunctionAttribute.java @@ -8,9 +8,10 @@ package org.elasticsearch.xpack.sql.expression.function.aggregate; import org.elasticsearch.xpack.sql.expression.Attribute; import org.elasticsearch.xpack.sql.expression.Expression; import org.elasticsearch.xpack.sql.expression.ExpressionId; +import org.elasticsearch.xpack.sql.expression.Nullability; import org.elasticsearch.xpack.sql.expression.function.FunctionAttribute; -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 java.util.Objects; @@ -21,12 +22,12 @@ public class AggregateFunctionAttribute extends FunctionAttribute { AggregateFunctionAttribute(Source source, String name, DataType dataType, ExpressionId id, String functionId, String propertyPath) { - this(source, name, dataType, null, false, id, false, functionId, propertyPath); + this(source, name, dataType, null, Nullability.FALSE, id, false, functionId, propertyPath); } public AggregateFunctionAttribute(Source source, String name, DataType dataType, String qualifier, - boolean nullable, ExpressionId id, boolean synthetic, String functionId, String propertyPath) { - super(source, name, dataType, qualifier, nullable, id, synthetic, functionId); + Nullability nullability, ExpressionId id, boolean synthetic, String functionId, String propertyPath) { + super(source, name, dataType, qualifier, nullability, id, synthetic, functionId); this.propertyPath = propertyPath; } @@ -42,14 +43,14 @@ public class AggregateFunctionAttribute extends FunctionAttribute { @Override protected Expression canonicalize() { - return new AggregateFunctionAttribute(source(), "", dataType(), null, true, id(), false, "", null); + return new AggregateFunctionAttribute(source(), "", dataType(), null, Nullability.TRUE, id(), false, "", null); } @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) { // this is highly correlated with QueryFolder$FoldAggregate#addFunction (regarding the function name within the querydsl) // that is the functionId is actually derived from the expression id to easily track it across contexts - return new AggregateFunctionAttribute(source, name, dataType(), qualifier, nullable, id, synthetic, functionId(), propertyPath); + return new AggregateFunctionAttribute(source, name, dataType(), qualifier, nullability, id, synthetic, functionId(), propertyPath); } public AggregateFunctionAttribute withFunctionId(String functionId, String propertyPath) { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/grouping/GroupingFunctionAttribute.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/grouping/GroupingFunctionAttribute.java index 7649d33b3a9..c33c893141b 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/grouping/GroupingFunctionAttribute.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/grouping/GroupingFunctionAttribute.java @@ -8,20 +8,21 @@ package org.elasticsearch.xpack.sql.expression.function.grouping; import org.elasticsearch.xpack.sql.expression.Attribute; import org.elasticsearch.xpack.sql.expression.Expression; import org.elasticsearch.xpack.sql.expression.ExpressionId; +import org.elasticsearch.xpack.sql.expression.Nullability; import org.elasticsearch.xpack.sql.expression.function.FunctionAttribute; -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 GroupingFunctionAttribute extends FunctionAttribute { GroupingFunctionAttribute(Source source, String name, DataType dataType, ExpressionId id, String functionId) { - this(source, name, dataType, null, false, id, false, functionId); + this(source, name, dataType, null, Nullability.FALSE, id, false, functionId); } public GroupingFunctionAttribute(Source source, String name, DataType dataType, String qualifier, - boolean nullable, ExpressionId id, boolean synthetic, String functionId) { - super(source, name, dataType, qualifier, nullable, id, synthetic, functionId); + Nullability nullability, ExpressionId id, boolean synthetic, String functionId) { + super(source, name, dataType, qualifier, nullability, id, synthetic, functionId); } @Override @@ -32,14 +33,15 @@ public class GroupingFunctionAttribute extends FunctionAttribute { @Override protected Expression canonicalize() { - return new GroupingFunctionAttribute(source(), "", dataType(), null, true, id(), false, ""); + return new GroupingFunctionAttribute(source(), "", dataType(), 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) { // this is highly correlated with QueryFolder$FoldAggregate#addFunction (regarding the function name within the querydsl) // that is the functionId is actually derived from the expression id to easily track it across contexts - return new GroupingFunctionAttribute(source, name, dataType(), qualifier, nullable, id, synthetic, functionId()); + return new GroupingFunctionAttribute(source, name, dataType(), qualifier, nullability, id, synthetic, functionId()); } public GroupingFunctionAttribute withFunctionId(String functionId, String propertyPath) { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Cast.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Cast.java index 1414e07da0c..7281476b1f9 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Cast.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Cast.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.sql.expression.function.scalar; import org.elasticsearch.xpack.sql.expression.Expression; +import org.elasticsearch.xpack.sql.expression.Nullability; import org.elasticsearch.xpack.sql.expression.gen.processor.Processor; import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate; import org.elasticsearch.xpack.sql.tree.Source; @@ -62,8 +63,11 @@ public class Cast extends UnaryScalarFunction { } @Override - public boolean nullable() { - return field().nullable() || DataTypes.isNull(from()); + public Nullability nullable() { + if (DataTypes.isNull(from())) { + return Nullability.TRUE; + } + return field().nullable(); } @Override diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/ConfigurationFunction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/ConfigurationFunction.java index 949cded29d2..39ee00d9b72 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/ConfigurationFunction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/ConfigurationFunction.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.sql.expression.function.scalar; import org.elasticsearch.xpack.sql.expression.Expression; +import org.elasticsearch.xpack.sql.expression.Nullability; import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate; import org.elasticsearch.xpack.sql.session.Configuration; import org.elasticsearch.xpack.sql.tree.Source; @@ -42,8 +43,8 @@ public abstract class ConfigurationFunction extends ScalarFunction { } @Override - public boolean nullable() { - return false; + public Nullability nullable() { + return Nullability.FALSE; } @Override @@ -73,4 +74,4 @@ public abstract class ConfigurationFunction extends ScalarFunction { public boolean equals(Object obj) { return super.equals(obj) && Objects.equals(fold(), ((ConfigurationFunction) obj).fold()); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/ScalarFunctionAttribute.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/ScalarFunctionAttribute.java index c4b91415a02..6a0980c2690 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/ScalarFunctionAttribute.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/ScalarFunctionAttribute.java @@ -8,11 +8,12 @@ package org.elasticsearch.xpack.sql.expression.function.scalar; import org.elasticsearch.xpack.sql.expression.Attribute; import org.elasticsearch.xpack.sql.expression.Expression; import org.elasticsearch.xpack.sql.expression.ExpressionId; +import org.elasticsearch.xpack.sql.expression.Nullability; import org.elasticsearch.xpack.sql.expression.function.FunctionAttribute; import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe; 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 java.util.Objects; @@ -25,13 +26,13 @@ public class ScalarFunctionAttribute extends FunctionAttribute { ScalarFunctionAttribute(Source source, String name, DataType dataType, ExpressionId id, String functionId, ScriptTemplate script, Expression orderBy, Pipe processorDef) { - this(source, name, dataType, null, true, id, false, functionId, script, orderBy, processorDef); + this(source, name, dataType, null, Nullability.TRUE, id, false, functionId, script, orderBy, processorDef); } public ScalarFunctionAttribute(Source source, String name, DataType dataType, String qualifier, - boolean nullable, ExpressionId id, boolean synthetic, String functionId, ScriptTemplate script, - Expression orderBy, Pipe pipe) { - super(source, name, dataType, qualifier, nullable, id, synthetic, functionId); + Nullability nullability, ExpressionId id, boolean synthetic, String functionId, ScriptTemplate script, + Expression orderBy, Pipe pipe) { + super(source, name, dataType, qualifier, nullability, id, synthetic, functionId); this.script = script; this.orderBy = orderBy; @@ -60,14 +61,15 @@ public class ScalarFunctionAttribute extends FunctionAttribute { @Override protected Expression canonicalize() { - return new ScalarFunctionAttribute(source(), "", dataType(), null, true, id(), false, + return new ScalarFunctionAttribute(source(), "", dataType(), null, Nullability.TRUE, id(), false, functionId(), script, orderBy, pipe); } @Override - protected Attribute clone(Source source, String name, String qualifier, boolean nullable, ExpressionId id, boolean synthetic) { - return new ScalarFunctionAttribute(source, name, dataType(), qualifier, nullable, id, synthetic, - functionId(), script, orderBy, pipe); + protected Attribute clone(Source source, String name, String qualifier, Nullability nullability, + ExpressionId id, boolean synthetic) { + return new ScalarFunctionAttribute(source, name, dataType(), qualifier, nullability, + id, synthetic, functionId(), script, orderBy, pipe); } @Override @@ -90,4 +92,4 @@ public class ScalarFunctionAttribute extends FunctionAttribute { protected String label() { return "s->" + functionId(); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Concat.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Concat.java index 95b6efb02fb..9a132c012a8 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Concat.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Concat.java @@ -9,6 +9,7 @@ import org.elasticsearch.xpack.sql.expression.Expression; import org.elasticsearch.xpack.sql.expression.Expressions; import org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal; import org.elasticsearch.xpack.sql.expression.FieldAttribute; +import org.elasticsearch.xpack.sql.expression.Nullability; import org.elasticsearch.xpack.sql.expression.function.scalar.BinaryScalarFunction; import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe; import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate; @@ -50,8 +51,8 @@ public class Concat extends BinaryScalarFunction { } @Override - public boolean nullable() { - return false; + public Nullability nullable() { + return Nullability.FALSE; } @Override @@ -85,4 +86,4 @@ public class Concat extends BinaryScalarFunction { public DataType dataType() { return DataType.KEYWORD; } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/Range.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/Range.java index e9b3b582b1e..a189b7fda0e 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/Range.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/Range.java @@ -8,6 +8,7 @@ package org.elasticsearch.xpack.sql.expression.predicate; import org.elasticsearch.xpack.sql.expression.Expression; import org.elasticsearch.xpack.sql.expression.Expressions; import org.elasticsearch.xpack.sql.expression.Literal; +import org.elasticsearch.xpack.sql.expression.Nullability; 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.Params; @@ -119,8 +120,8 @@ public class Range extends ScalarFunction { } @Override - public boolean nullable() { - return value.nullable() && lower.nullable() && upper.nullable(); + public Nullability nullable() { + return Nullability.and(value.nullable(), lower.nullable(), upper.nullable()); } @Override @@ -216,4 +217,4 @@ public class Range extends ScalarFunction { public String toString() { return name(); } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/ConditionalFunction.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/ConditionalFunction.java index c89e29beb6f..13b765e941c 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/ConditionalFunction.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/ConditionalFunction.java @@ -8,6 +8,7 @@ package org.elasticsearch.xpack.sql.expression.predicate.conditional; import org.elasticsearch.xpack.sql.expression.Expression; import org.elasticsearch.xpack.sql.expression.Expressions; +import org.elasticsearch.xpack.sql.expression.Nullability; import org.elasticsearch.xpack.sql.expression.function.scalar.ScalarFunction; import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.type.DataType; @@ -36,7 +37,7 @@ public abstract class ConditionalFunction extends ScalarFunction { } @Override - public boolean nullable() { - return false; + public Nullability nullable() { + return Nullability.UNKNOWN; } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/NullIf.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/NullIf.java index 84f59851b39..ef1a71c7ed4 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/NullIf.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/conditional/NullIf.java @@ -8,6 +8,7 @@ package org.elasticsearch.xpack.sql.expression.predicate.conditional; import org.elasticsearch.xpack.sql.expression.Expression; import org.elasticsearch.xpack.sql.expression.Expressions; +import org.elasticsearch.xpack.sql.expression.Nullability; import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe; import org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder; import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate; @@ -57,8 +58,8 @@ public class NullIf extends ConditionalFunction { } @Override - public boolean nullable() { - return true; + public Nullability nullable() { + return Nullability.UNKNOWN; } @Override diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/fulltext/FullTextPredicate.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/fulltext/FullTextPredicate.java index e8f2f880c56..7af299b97f5 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/fulltext/FullTextPredicate.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/fulltext/FullTextPredicate.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.sql.expression.predicate.fulltext; import org.elasticsearch.xpack.sql.expression.Expression; +import org.elasticsearch.xpack.sql.expression.Nullability; import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.type.DataType; @@ -56,8 +57,8 @@ public abstract class FullTextPredicate extends Expression { } @Override - public boolean nullable() { - return false; + public Nullability nullable() { + return Nullability.FALSE; } @Override @@ -84,4 +85,4 @@ public abstract class FullTextPredicate extends Expression { return Objects.equals(query, other.query) && Objects.equals(options, other.options); } -} \ No newline at end of file +} 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 a1936a86692..d4bce9feab3 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 @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.sql.expression.predicate.logical; import org.elasticsearch.xpack.sql.expression.Expression; import org.elasticsearch.xpack.sql.expression.Expressions; +import org.elasticsearch.xpack.sql.expression.Nullability; import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe; import org.elasticsearch.xpack.sql.expression.predicate.BinaryOperator; import org.elasticsearch.xpack.sql.expression.predicate.logical.BinaryLogicProcessor.BinaryLogicOperation; @@ -35,8 +36,8 @@ public abstract class BinaryLogic extends BinaryOperator { @Override protected Expression rule(Expression e) { if (e instanceof IsNotNull) { - if (((IsNotNull) e).field().nullable() == false) { + if (((IsNotNull) e).field().nullable() == Nullability.FALSE) { return new Literal(e.source(), Expressions.name(e), Boolean.TRUE, DataType.BOOLEAN); } } else if (e instanceof IsNull) { - if (((IsNull) e).field().nullable() == false) { + if (((IsNull) e).field().nullable() == Nullability.FALSE) { return new Literal(e.source(), Expressions.name(e), Boolean.FALSE, DataType.BOOLEAN); } @@ -1112,10 +1112,7 @@ public class Optimizer extends RuleExecutor { return Literal.of(in, null); } - } else if (e instanceof NullIf) { - return e; - - } else if (e.nullable() && Expressions.anyMatch(e.children(), Expressions::isNull)) { + } else if (e.nullable() == Nullability.TRUE && Expressions.anyMatch(e.children(), Expressions::isNull)) { return Literal.of(e, null); } @@ -1314,7 +1311,7 @@ public class Optimizer extends RuleExecutor { // true for equality if (bc instanceof Equals || bc instanceof GreaterThanOrEqual || bc instanceof LessThanOrEqual) { - if (!l.nullable() && !r.nullable() && l.semanticEquals(r)) { + if (l.nullable() == Nullability.FALSE && r.nullable() == Nullability.FALSE && l.semanticEquals(r)) { return TRUE; } } @@ -1329,7 +1326,7 @@ public class Optimizer extends RuleExecutor { // false for equality if (bc instanceof NotEquals || bc instanceof GreaterThan || bc instanceof LessThan) { - if (!l.nullable() && !r.nullable() && l.semanticEquals(r)) { + if (l.nullable() == Nullability.FALSE && r.nullable() == Nullability.FALSE && l.semanticEquals(r)) { return FALSE; } } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/Join.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/Join.java index 397755d7051..f09b43ec41d 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/Join.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/Join.java @@ -5,17 +5,17 @@ */ package org.elasticsearch.xpack.sql.plan.logical; +import org.elasticsearch.xpack.sql.expression.Attribute; +import org.elasticsearch.xpack.sql.expression.Expression; +import org.elasticsearch.xpack.sql.expression.Nullability; +import org.elasticsearch.xpack.sql.tree.NodeInfo; +import org.elasticsearch.xpack.sql.tree.Source; +import org.elasticsearch.xpack.sql.type.DataType; + import java.util.List; import java.util.Objects; -import org.elasticsearch.xpack.sql.expression.Attribute; -import org.elasticsearch.xpack.sql.expression.Expression; -import org.elasticsearch.xpack.sql.tree.Source; -import org.elasticsearch.xpack.sql.tree.NodeInfo; -import org.elasticsearch.xpack.sql.type.DataType; - import static java.util.stream.Collectors.toList; - import static org.elasticsearch.xpack.sql.util.CollectionUtils.combine; public class Join extends BinaryPlan { @@ -78,7 +78,7 @@ public class Join extends BinaryPlan { private static List makeNullable(List output) { return output.stream() - .map(a -> a.withNullability(true)) + .map(a -> a.withNullability(Nullability.TRUE)) .collect(toList()); } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/NullabilityTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/NullabilityTests.java new file mode 100644 index 00000000000..1b493de6b4c --- /dev/null +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/NullabilityTests.java @@ -0,0 +1,35 @@ +/* + * 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; + +import org.elasticsearch.test.ESTestCase; + +import static org.elasticsearch.xpack.sql.expression.Nullability.FALSE; +import static org.elasticsearch.xpack.sql.expression.Nullability.TRUE; +import static org.elasticsearch.xpack.sql.expression.Nullability.UNKNOWN; + +public class NullabilityTests extends ESTestCase { + + public void testLogicalAndOfNullabilities() { + assertEquals(FALSE, Nullability.and()); + + assertEquals(TRUE, Nullability.and(TRUE)); + assertEquals(FALSE, Nullability.and(FALSE)); + assertEquals(UNKNOWN, Nullability.and(UNKNOWN)); + + assertEquals(UNKNOWN, Nullability.and(UNKNOWN, UNKNOWN)); + assertEquals(UNKNOWN, Nullability.and(UNKNOWN, TRUE)); + assertEquals(UNKNOWN, Nullability.and(UNKNOWN, FALSE)); + + assertEquals(FALSE, Nullability.and(FALSE, FALSE)); + assertEquals(TRUE, Nullability.and(FALSE, TRUE)); + assertEquals(UNKNOWN, Nullability.and(FALSE, UNKNOWN)); + + assertEquals(TRUE, Nullability.and(TRUE, TRUE)); + assertEquals(TRUE, Nullability.and(TRUE, FALSE)); + assertEquals(UNKNOWN, Nullability.and(TRUE, UNKNOWN)); + } +} diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java index 9629015d29b..bc5bc0985cb 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java @@ -14,6 +14,7 @@ import org.elasticsearch.xpack.sql.expression.FieldAttribute; import org.elasticsearch.xpack.sql.expression.Foldables; import org.elasticsearch.xpack.sql.expression.Literal; import org.elasticsearch.xpack.sql.expression.NamedExpression; +import org.elasticsearch.xpack.sql.expression.Nullability; import org.elasticsearch.xpack.sql.expression.Order; import org.elasticsearch.xpack.sql.expression.Order.OrderDirection; import org.elasticsearch.xpack.sql.expression.function.Function; @@ -134,8 +135,8 @@ public class OptimizerTests extends ESTestCase { } @Override - public boolean nullable() { - return false; + public Nullability nullable() { + return Nullability.FALSE; } @Override @@ -393,6 +394,8 @@ public class OptimizerTests extends ESTestCase { return ((Literal) new ConstantFolding().rule(b)).value(); } + // Null folding + public void testNullFoldingIsNull() { FoldNull foldNull = new FoldNull(); assertEquals(true, foldNull.rule(new IsNull(EMPTY, Literal.NULL)).fold()); @@ -422,6 +425,34 @@ public class OptimizerTests extends ESTestCase { assertNullLiteral(rule.rule(new RLike(EMPTY, Literal.NULL, "123"))); } + public void testNullFoldingDoesNotApplyOnLogicalExpressions() { + FoldNull rule = new FoldNull(); + + Or or = new Or(EMPTY, Literal.NULL, Literal.TRUE); + assertEquals(or, rule.rule(or)); + or = new Or(EMPTY, Literal.NULL, Literal.NULL); + assertEquals(or, rule.rule(or)); + + And and = new And(EMPTY, Literal.NULL, Literal.TRUE); + assertEquals(and, rule.rule(and)); + and = new And(EMPTY, Literal.NULL, Literal.NULL); + assertEquals(and, rule.rule(and)); + } + + public void testNullFoldingDoesNotApplyOnConditionals() { + FoldNull rule = new FoldNull(); + + Coalesce coalesce = new Coalesce(EMPTY, Arrays.asList(Literal.NULL, ONE, TWO)); + assertEquals(coalesce, rule.rule(coalesce)); + coalesce = new Coalesce(EMPTY, Arrays.asList(Literal.NULL, NULL, NULL)); + assertEquals(coalesce, rule.rule(coalesce)); + + Greatest greatest = new Greatest(EMPTY, Arrays.asList(Literal.NULL, ONE, TWO)); + assertEquals(greatest, rule.rule(greatest)); + greatest = new Greatest(EMPTY, Arrays.asList(Literal.NULL, ONE, TWO)); + assertEquals(greatest, rule.rule(greatest)); + } + public void testSimplifyCoalesceNulls() { Expression e = new SimplifyConditional().rule(new Coalesce(EMPTY, asList(Literal.NULL, Literal.NULL))); assertEquals(Coalesce.class, e.getClass());