SQL: Fix function args verification and error msgs (#34926)

Extract data type verification for function arguments to a single place
so that NULL type can be treated as RESOLVED for all functions. Moreover
this enables to have consistent verification error messages for all functions.

Fixes: #34752
Fixes: #33469

x-pack/plugin/sql/sql-proto/src/main/java/org/elasticsearch/xpack/sql/type/DataType.java

@@ -246,4 +246,4 @@ public enum DataType {
             (isString() && other.isString()) ||
             (isNumeric() && other.isNumeric());
     }
-}
+}

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

@@ -35,7 +35,11 @@ public abstract class Expression extends Node<Expression> implements Resolvable
 
         public static final TypeResolution TYPE_RESOLVED = new TypeResolution(false, StringUtils.EMPTY);
 
-        public TypeResolution(String message, Object... args) {
+        public TypeResolution(String message) {
+            this(true, message);
+        }
+
+        TypeResolution(String message, Object... args) {
             this(true, format(Locale.ROOT, message, args));
         }
 
@@ -132,4 +136,4 @@ public abstract class Expression extends Node<Expression> implements Resolvable
     public String toString() {
         return nodeName() + "[" + propertiesToString(false) + "]";
     }
-}
+}

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

@@ -10,10 +10,12 @@ import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
 import org.elasticsearch.xpack.sql.expression.Expression.TypeResolution;
 import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe;
 import org.elasticsearch.xpack.sql.type.DataType;
+import org.elasticsearch.xpack.sql.type.DataTypes;
 
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.Locale;
 import java.util.function.Predicate;
 
 import static java.util.Collections.emptyList;
@@ -21,6 +23,14 @@ import static java.util.Collections.emptyMap;
 
 public final class Expressions {
 
+    public enum ParamOrdinal {
+        DEFAULT,
+        FIRST,
+        SECOND,
+        THIRD,
+        FOURTH
+    }
+
     private Expressions() {}
 
     public static NamedExpression wrapAsNamed(Expression exp) {
@@ -127,22 +137,51 @@ public final class Expressions {
         throw new SqlIllegalArgumentException("Cannot create pipe for {}", e);
     }
 
-    public static TypeResolution typeMustBe(Expression e, Predicate<Expression> predicate, String message) {
+    public static TypeResolution typeMustBeBoolean(Expression e, String operationName, ParamOrdinal paramOrd) {
-        return predicate.test(e) ? TypeResolution.TYPE_RESOLVED : new TypeResolution(message);
+        return typeMustBe(e, dt -> dt == DataType.BOOLEAN, operationName, paramOrd, "boolean");
     }
 
-    public static TypeResolution typeMustBeNumeric(Expression e) {
+    public static TypeResolution typeMustBeInteger(Expression e, String operationName, ParamOrdinal paramOrd) {
-        return e.dataType().isNumeric() ? TypeResolution.TYPE_RESOLVED : new TypeResolution(incorrectTypeErrorMessage(e, "numeric"));
+        return typeMustBe(e, dt -> dt.isInteger, operationName, paramOrd, "integer");
     }
 
-    public static TypeResolution typeMustBeNumericOrDate(Expression e) {
+    public static TypeResolution typeMustBeNumeric(Expression e, String operationName, ParamOrdinal paramOrd) {
-        return e.dataType().isNumeric() || e.dataType() == DataType.DATE ?
+        return typeMustBe(e, DataType::isNumeric, operationName, paramOrd, "numeric");
+    }
+
+    public static TypeResolution typeMustBeString(Expression e, String operationName, ParamOrdinal paramOrd) {
+        return typeMustBe(e, DataType::isString, operationName, paramOrd, "string");
+    }
+
+    public static TypeResolution typeMustBeDate(Expression e, String operationName, ParamOrdinal paramOrd) {
+        return typeMustBe(e, dt -> dt == DataType.DATE, operationName, paramOrd, "date");
+    }
+
+    public static TypeResolution typeMustBeNumericOrDate(Expression e, String operationName, ParamOrdinal paramOrd) {
+        return typeMustBe(e, dt -> dt.isNumeric() || dt == DataType.DATE, operationName, paramOrd, "numeric", "date");
+    }
+
+    private static TypeResolution typeMustBe(Expression e,
+                                             Predicate<DataType> predicate,
+                                             String operationName,
+                                             ParamOrdinal pOrd,
+                                             String... acceptedTypes) {
+
+        return predicate.test(e.dataType()) || DataTypes.isNull(e.dataType())?
             TypeResolution.TYPE_RESOLVED :
-                new TypeResolution(incorrectTypeErrorMessage(e, "numeric", "date"));
+            new TypeResolution(incorrectTypeErrorMessage(e, operationName, pOrd, acceptedTypes));
+
     }
-    
+
-    private static String incorrectTypeErrorMessage(Expression e, String...acceptedTypes) {
+    private static String incorrectTypeErrorMessage(Expression e,
-        return "Argument required to be " + Strings.arrayToDelimitedString(acceptedTypes, " or ")
+                                                    String operationName,
-                + " ('" + Expressions.name(e) + "' type is '" + e.dataType().esType + "')";
+                                                    ParamOrdinal paramOrd,
+                                                    String... acceptedTypes) {
+        return String.format(Locale.ROOT, "[%s]%s argument must be [%s], found value [%s] type [%s]",
+            operationName,
+            paramOrd == null || paramOrd == ParamOrdinal.DEFAULT ? "" : " " + paramOrd.name().toLowerCase(Locale.ROOT),
+            Strings.arrayToDelimitedString(acceptedTypes, " or "),
+            Expressions.name(e),
+            e.dataType().esType);
     }
-}
+}

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

@@ -26,6 +26,7 @@ public class Literal extends NamedExpression {
 
     public static final Literal TRUE = Literal.of(Location.EMPTY, Boolean.TRUE);
     public static final Literal FALSE = Literal.of(Location.EMPTY, Boolean.FALSE);
+    public static final Literal NULL = Literal.of(Location.EMPTY, null);
 
     private final Object value;
     private final DataType dataType;
@@ -163,4 +164,4 @@ public class Literal extends NamedExpression {
 
         return new Literal(foldable.location(), name, fold, foldable.dataType());
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Max.java

@@ -7,6 +7,7 @@ package org.elasticsearch.xpack.sql.expression.function.aggregate;
 
 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.tree.Location;
 import org.elasticsearch.xpack.sql.tree.NodeInfo;
 import org.elasticsearch.xpack.sql.type.DataType;
@@ -44,6 +45,6 @@ public class Max extends NumericAggregate implements EnclosedAgg {
 
     @Override
     protected TypeResolution resolveType() {
-        return Expressions.typeMustBeNumericOrDate(field());
+        return Expressions.typeMustBeNumericOrDate(field(), functionName(), ParamOrdinal.DEFAULT);
     }
 }

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Min.java

@@ -7,6 +7,7 @@ package org.elasticsearch.xpack.sql.expression.function.aggregate;
 
 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.tree.Location;
 import org.elasticsearch.xpack.sql.tree.NodeInfo;
 import org.elasticsearch.xpack.sql.type.DataType;
@@ -47,6 +48,6 @@ public class Min extends NumericAggregate implements EnclosedAgg {
 
     @Override
     protected TypeResolution resolveType() {
-        return Expressions.typeMustBeNumericOrDate(field());
+        return Expressions.typeMustBeNumericOrDate(field(), functionName(), ParamOrdinal.DEFAULT);
     }
 }

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/NumericAggregate.java

@@ -7,6 +7,7 @@ package org.elasticsearch.xpack.sql.expression.function.aggregate;
 
 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.tree.Location;
 import org.elasticsearch.xpack.sql.type.DataType;
 
@@ -24,7 +25,7 @@ abstract class NumericAggregate extends AggregateFunction {
 
     @Override
     protected TypeResolution resolveType() {
-        return Expressions.typeMustBeNumeric(field());
+        return Expressions.typeMustBeNumeric(field(), functionName(), ParamOrdinal.DEFAULT);
     }
 
     @Override

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/Percentile.java

@@ -7,6 +7,7 @@ package org.elasticsearch.xpack.sql.expression.function.aggregate;
 
 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.Foldables;
 import org.elasticsearch.xpack.sql.tree.Location;
 import org.elasticsearch.xpack.sql.tree.NodeInfo;
@@ -43,7 +44,7 @@ public class Percentile extends NumericAggregate implements EnclosedAgg {
         TypeResolution resolution = super.resolveType();
 
         if (TypeResolution.TYPE_RESOLVED.equals(resolution)) {
-            resolution = Expressions.typeMustBeNumeric(percent());
+            resolution = Expressions.typeMustBeNumeric(percent(), functionName(), ParamOrdinal.DEFAULT);
         }
 
         return resolution;

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/aggregate/PercentileRank.java

@@ -7,6 +7,7 @@ package org.elasticsearch.xpack.sql.expression.function.aggregate;
 
 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.Foldables;
 import org.elasticsearch.xpack.sql.tree.Location;
 import org.elasticsearch.xpack.sql.tree.NodeInfo;
@@ -41,12 +42,11 @@ public class PercentileRank extends AggregateFunction implements EnclosedAgg {
     @Override
     protected TypeResolution resolveType() {
         TypeResolution resolution = super.resolveType();
-
+        if (resolution.unresolved()) {
-        if (TypeResolution.TYPE_RESOLVED.equals(resolution)) {
+            return resolution;
-            resolution = Expressions.typeMustBeNumeric(value);
         }
 
-        return resolution;
+        return Expressions.typeMustBeNumeric(value, functionName(), ParamOrdinal.DEFAULT);
     }
 
     public Expression value() {

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/Cast.java

@@ -65,7 +65,7 @@ public class Cast extends UnaryScalarFunction {
     protected TypeResolution resolveType() {
         return DataTypeConversion.canConvert(from(), to()) ?
                 TypeResolution.TYPE_RESOLVED :
-                    new TypeResolution("Cannot cast %s to %s", from(), to());
+                    new TypeResolution("Cannot cast [" + from() + "] to [" + to()+ "]");
     }
 
     @Override
@@ -102,4 +102,4 @@ public class Cast extends UnaryScalarFunction {
         sb.insert(sb.length() - 1, " AS " + to().sqlName());
         return sb.toString();
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/BaseDateTimeFunction.java

@@ -8,10 +8,10 @@ package org.elasticsearch.xpack.sql.expression.function.scalar.datetime;
 
 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.function.scalar.UnaryScalarFunction;
 import org.elasticsearch.xpack.sql.tree.Location;
 import org.elasticsearch.xpack.sql.tree.NodeInfo;
-import org.elasticsearch.xpack.sql.type.DataType;
 import org.joda.time.DateTime;
 
 import java.util.Objects;
@@ -42,11 +42,7 @@ abstract class BaseDateTimeFunction extends UnaryScalarFunction {
 
     @Override
     protected TypeResolution resolveType() {
-        if (field().dataType() == DataType.DATE) {
+        return Expressions.typeMustBeDate(field(), functionName(), ParamOrdinal.DEFAULT);
-            return TypeResolution.TYPE_RESOLVED;
-        }
-        return new TypeResolution("Function [" + functionName() + "] cannot be applied on a non-date expression (["
-                + Expressions.name(field()) + "] of type [" + field().dataType().esType + "])");
     }
 
     public TimeZone timeZone() {
@@ -90,4 +86,4 @@ abstract class BaseDateTimeFunction extends UnaryScalarFunction {
     public int hashCode() {
         return Objects.hash(field(), timeZone());
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/BinaryNumericFunction.java

@@ -7,6 +7,7 @@ package org.elasticsearch.xpack.sql.expression.function.scalar.math;
 
 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.function.scalar.BinaryScalarFunction;
 import org.elasticsearch.xpack.sql.expression.function.scalar.math.BinaryMathProcessor.BinaryMathOperation;
 import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe;
@@ -19,7 +20,7 @@ public abstract class BinaryNumericFunction extends BinaryScalarFunction {
 
     private final BinaryMathOperation operation;
 
-    protected BinaryNumericFunction(Location location, Expression left, Expression right, BinaryMathOperation operation) {
+    BinaryNumericFunction(Location location, Expression left, Expression right, BinaryMathOperation operation) {
         super(location, left, right);
         this.operation = operation;
     }
@@ -35,18 +36,12 @@ public abstract class BinaryNumericFunction extends BinaryScalarFunction {
             return new TypeResolution("Unresolved children");
         }
 
-        TypeResolution resolution = resolveInputType(left().dataType());
+        TypeResolution resolution = Expressions.typeMustBeNumeric(left(), functionName(), ParamOrdinal.FIRST);
+        if (resolution.unresolved()) {
+            return resolution;
 
-        if (resolution == TypeResolution.TYPE_RESOLVED) {
-            return resolveInputType(right().dataType());
         }
-        return resolution;
+        return Expressions.typeMustBeNumeric(right(), functionName(), ParamOrdinal.SECOND);
-    }
-
-    protected TypeResolution resolveInputType(DataType inputType) {
-        return inputType.isNumeric() ?
-                TypeResolution.TYPE_RESOLVED :
-                new TypeResolution("'%s' requires a numeric type, received %s", scriptMethodName(), inputType.esType);
     }
 
     @Override
@@ -74,4 +69,4 @@ public abstract class BinaryNumericFunction extends BinaryScalarFunction {
             && Objects.equals(other.right(), right())
                 && Objects.equals(other.operation, operation);
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/math/MathFunction.java

@@ -6,6 +6,8 @@
 package org.elasticsearch.xpack.sql.expression.function.scalar.math;
 
 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.function.scalar.UnaryScalarFunction;
 import org.elasticsearch.xpack.sql.expression.function.scalar.math.MathProcessor.MathOperation;
 import org.elasticsearch.xpack.sql.expression.gen.processor.Processor;
@@ -57,8 +59,7 @@ public abstract class MathFunction extends UnaryScalarFunction {
             return new TypeResolution("Unresolved children");
         }
 
-        return field().dataType().isNumeric() ? TypeResolution.TYPE_RESOLVED
+        return Expressions.typeMustBeNumeric(field(), operation().toString(), ParamOrdinal.DEFAULT);
-                : new TypeResolution("'%s' requires a numeric type, received %s", operation(), field().dataType().esType);
     }
 
     @Override
@@ -81,4 +82,4 @@ public abstract class MathFunction extends UnaryScalarFunction {
     public int hashCode() {
         return Objects.hash(field());
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringFunction.java

@@ -10,12 +10,13 @@ import org.elasticsearch.xpack.sql.expression.FieldAttribute;
 import org.elasticsearch.xpack.sql.expression.function.scalar.BinaryScalarFunction;
 import org.elasticsearch.xpack.sql.expression.gen.script.ScriptTemplate;
 import org.elasticsearch.xpack.sql.tree.Location;
-import org.elasticsearch.xpack.sql.type.DataType;
 
 import java.util.Locale;
 import java.util.Objects;
 import java.util.function.BiFunction;
 
+import static org.elasticsearch.xpack.sql.expression.Expressions.ParamOrdinal;
+import static org.elasticsearch.xpack.sql.expression.Expressions.typeMustBeString;
 import static org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder.paramsBuilder;
 
 /**
@@ -41,14 +42,15 @@ public abstract class BinaryStringFunction<T,R> extends BinaryScalarFunction {
             return new TypeResolution("Unresolved children");
         }
 
-        if (!left().dataType().isString()) {
+        TypeResolution resolution = typeMustBeString(left(), functionName(), ParamOrdinal.FIRST);
-            return new TypeResolution("'%s' requires first parameter to be a string type, received %s", functionName(), left().dataType());
+        if (resolution.unresolved()) {
+            return resolution;
         }
-                
+
-        return resolveSecondParameterInputType(right().dataType());
+        return resolveSecondParameterInputType(right());
     }
 
-    protected abstract TypeResolution resolveSecondParameterInputType(DataType inputType);
+    protected abstract TypeResolution resolveSecondParameterInputType(Expression e);
 
     @Override
     public Object fold() {
@@ -83,4 +85,4 @@ public abstract class BinaryStringFunction<T,R> extends BinaryScalarFunction {
         return Objects.equals(other.left(), left())
             && Objects.equals(other.right(), right());
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringNumericFunction.java

@@ -25,10 +25,8 @@ public abstract class BinaryStringNumericFunction extends BinaryStringFunction<N
     protected abstract BinaryStringNumericOperation operation();
 
     @Override
-    protected TypeResolution resolveSecondParameterInputType(DataType inputType) {
+    protected TypeResolution resolveSecondParameterInputType(Expression e) {
-        return inputType.isNumeric() ?
+        return Expressions.typeMustBeNumeric(e,functionName(), Expressions.ParamOrdinal.SECOND);
-                TypeResolution.TYPE_RESOLVED :
-                new TypeResolution("'%s' requires second parameter to be a numeric type, received %s", functionName(), inputType);
     }
 
     @Override

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringStringFunction.java

@@ -6,6 +6,7 @@
 package org.elasticsearch.xpack.sql.expression.function.scalar.string;
 
 import org.elasticsearch.xpack.sql.expression.Expression;
+import org.elasticsearch.xpack.sql.expression.Expressions;
 import org.elasticsearch.xpack.sql.tree.Location;
 import org.elasticsearch.xpack.sql.type.DataType;
 
@@ -19,10 +20,8 @@ public abstract class BinaryStringStringFunction extends BinaryStringFunction<St
     }
 
     @Override
-    protected TypeResolution resolveSecondParameterInputType(DataType inputType) {
+    protected TypeResolution resolveSecondParameterInputType(Expression e) {
-        return inputType.isString() ? 
+        return Expressions.typeMustBeString(e, functionName(), Expressions.ParamOrdinal.SECOND);
-                TypeResolution.TYPE_RESOLVED : 
-                new TypeResolution("'%s' requires second parameter to be a string type, received %s", functionName(), inputType);
     }
 
     @Override

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Concat.java

@@ -7,6 +7,7 @@ package org.elasticsearch.xpack.sql.expression.function.scalar.string;
 
 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.function.scalar.BinaryScalarFunction;
 import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe;
@@ -35,12 +36,12 @@ public class Concat extends BinaryScalarFunction {
             return new TypeResolution("Unresolved children");
         }
 
-        TypeResolution sourceResolution = StringFunctionUtils.resolveStringInputType(left().dataType(), functionName());
+        TypeResolution sourceResolution = Expressions.typeMustBeString(left(), functionName(), ParamOrdinal.FIRST);
-        if (sourceResolution != TypeResolution.TYPE_RESOLVED) {
+        if (sourceResolution.unresolved()) {
             return sourceResolution;
         }
 
-        return StringFunctionUtils.resolveStringInputType(right().dataType(), functionName());
+        return Expressions.typeMustBeString(right(), functionName(), ParamOrdinal.SECOND);
     }
 
     @Override
@@ -79,4 +80,4 @@ public class Concat extends BinaryScalarFunction {
     public DataType dataType() {
         return DataType.KEYWORD;
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Insert.java

@@ -7,6 +7,7 @@ package org.elasticsearch.xpack.sql.expression.function.scalar.string;
 
 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.function.scalar.ScalarFunction;
 import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe;
@@ -45,22 +46,22 @@ public class Insert extends ScalarFunction {
             return new TypeResolution("Unresolved children");
         }
 
-        TypeResolution sourceResolution = StringFunctionUtils.resolveStringInputType(source.dataType(), functionName());
+        TypeResolution sourceResolution = Expressions.typeMustBeString(source, functionName(), ParamOrdinal.FIRST);
-        if (sourceResolution != TypeResolution.TYPE_RESOLVED) {
+        if (sourceResolution.unresolved()) {
             return sourceResolution;
         }
         
-        TypeResolution startResolution = StringFunctionUtils.resolveNumericInputType(start.dataType(), functionName());
+        TypeResolution startResolution = Expressions.typeMustBeNumeric(start, functionName(), ParamOrdinal.SECOND);
-        if (startResolution != TypeResolution.TYPE_RESOLVED) {
+        if (startResolution.unresolved()) {
             return startResolution;
         }
         
-        TypeResolution lengthResolution = StringFunctionUtils.resolveNumericInputType(length.dataType(), functionName());
+        TypeResolution lengthResolution = Expressions.typeMustBeNumeric(length, functionName(), ParamOrdinal.THIRD);
-        if (lengthResolution != TypeResolution.TYPE_RESOLVED) {
+        if (lengthResolution.unresolved()) {
             return lengthResolution;
         }
         
-        return StringFunctionUtils.resolveStringInputType(replacement.dataType(), functionName());
+        return Expressions.typeMustBeString(replacement, functionName(), ParamOrdinal.FOURTH);
     }
 
     @Override
@@ -135,4 +136,4 @@ public class Insert extends ScalarFunction {
 
         return new Insert(location(), newChildren.get(0), newChildren.get(1), newChildren.get(2), newChildren.get(3));
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Locate.java

@@ -7,6 +7,7 @@ package org.elasticsearch.xpack.sql.expression.function.scalar.string;
 
 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.function.scalar.ScalarFunction;
 import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe;
@@ -47,17 +48,19 @@ public class Locate extends ScalarFunction {
             return new TypeResolution("Unresolved children");
         }
 
-        TypeResolution patternResolution = StringFunctionUtils.resolveStringInputType(pattern.dataType(), functionName());
+        TypeResolution patternResolution = Expressions.typeMustBeString(pattern, functionName(), ParamOrdinal.FIRST);
-        if (patternResolution != TypeResolution.TYPE_RESOLVED) {
+        if (patternResolution.unresolved()) {
             return patternResolution;
         }
         
-        TypeResolution sourceResolution = StringFunctionUtils.resolveStringInputType(source.dataType(), functionName());
+        TypeResolution sourceResolution = Expressions.typeMustBeString(source, functionName(), ParamOrdinal.SECOND);
-        if (sourceResolution != TypeResolution.TYPE_RESOLVED) {
+        if (sourceResolution.unresolved()) {
             return sourceResolution;
         }
         
-        return start == null ? TypeResolution.TYPE_RESOLVED : StringFunctionUtils.resolveNumericInputType(start.dataType(), functionName());
+        return start == null ?
+            TypeResolution.TYPE_RESOLVED :
+            Expressions.typeMustBeNumeric(start, functionName(), ParamOrdinal.THIRD);
     }
 
     @Override
@@ -136,4 +139,4 @@ public class Locate extends ScalarFunction {
 
         return new Locate(location(), newChildren.get(0), newChildren.get(1), newChildren.get(2));
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Replace.java

@@ -7,6 +7,7 @@ package org.elasticsearch.xpack.sql.expression.function.scalar.string;
 
 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.function.scalar.ScalarFunction;
 import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe;
@@ -29,31 +30,31 @@ import static org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder.pa
 public class Replace extends ScalarFunction {
 
     private final Expression source, pattern, replacement;
-    
+
     public Replace(Location location, Expression source, Expression pattern, Expression replacement) {
         super(location, Arrays.asList(source, pattern, replacement));
         this.source = source;
         this.pattern = pattern;
         this.replacement = replacement;
     }
-    
+
     @Override
     protected TypeResolution resolveType() {
         if (!childrenResolved()) {
             return new TypeResolution("Unresolved children");
         }
 
-        TypeResolution sourceResolution = StringFunctionUtils.resolveStringInputType(source.dataType(), functionName());
+        TypeResolution sourceResolution = Expressions.typeMustBeString(source, functionName(), ParamOrdinal.FIRST);
-        if (sourceResolution != TypeResolution.TYPE_RESOLVED) {
+        if (sourceResolution.unresolved()) {
             return sourceResolution;
         }
-        
+
-        TypeResolution patternResolution = StringFunctionUtils.resolveStringInputType(pattern.dataType(), functionName());
+        TypeResolution patternResolution = Expressions.typeMustBeString(pattern, functionName(), ParamOrdinal.SECOND);
-        if (patternResolution != TypeResolution.TYPE_RESOLVED) {
+        if (patternResolution.unresolved()) {
             return patternResolution;
         }
-        
+
-        return StringFunctionUtils.resolveStringInputType(replacement.dataType(), functionName());
+        return Expressions.typeMustBeString(replacement, functionName(), ParamOrdinal.THIRD);
     }
 
     @Override
@@ -102,7 +103,7 @@ public class Replace extends ScalarFunction {
                     .script(replacementScript.params())
                     .build(), dataType());
     }
-    
+
     @Override
     public ScriptTemplate scriptWithField(FieldAttribute field) {
         return new ScriptTemplate(processScript("doc[{}].value"),
@@ -123,4 +124,4 @@ public class Replace extends ScalarFunction {
 
         return new Replace(location(), newChildren.get(0), newChildren.get(1), newChildren.get(2));
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringFunctionUtils.java

@@ -5,9 +5,6 @@
  */
 package org.elasticsearch.xpack.sql.expression.function.scalar.string;
 
-import org.elasticsearch.xpack.sql.expression.Expression.TypeResolution;
-import org.elasticsearch.xpack.sql.type.DataType;
-
 abstract class StringFunctionUtils {
 
     /**
@@ -74,15 +71,5 @@ abstract class StringFunctionUtils {
         return (s != null && s.length() > 0);
     }
     
-    static TypeResolution resolveStringInputType(DataType inputType, String functionName) {
+
-        return inputType.isString() ? 
-                TypeResolution.TYPE_RESOLVED : 
-                new TypeResolution("'%s' requires a string type, received %s", functionName, inputType.esType);
-    }
-    
-    static TypeResolution resolveNumericInputType(DataType inputType, String functionName) {
-        return inputType.isNumeric() ? 
-                TypeResolution.TYPE_RESOLVED : 
-                new TypeResolution("'%s' requires a numeric type, received %s", functionName, inputType.esType);
-    }
 }

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/Substring.java

@@ -7,6 +7,7 @@ package org.elasticsearch.xpack.sql.expression.function.scalar.string;
 
 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.function.scalar.ScalarFunction;
 import org.elasticsearch.xpack.sql.expression.gen.pipeline.Pipe;
@@ -44,17 +45,17 @@ public class Substring extends ScalarFunction {
             return new TypeResolution("Unresolved children");
         }
 
-        TypeResolution sourceResolution = StringFunctionUtils.resolveStringInputType(source.dataType(), functionName());
+        TypeResolution sourceResolution = Expressions.typeMustBeString(source, functionName(), ParamOrdinal.FIRST);
-        if (sourceResolution != TypeResolution.TYPE_RESOLVED) {
+        if (sourceResolution.unresolved()) {
             return sourceResolution;
         }
 
-        TypeResolution startResolution = StringFunctionUtils.resolveNumericInputType(start.dataType(), functionName());
+        TypeResolution startResolution = Expressions.typeMustBeNumeric(start, functionName(), ParamOrdinal.SECOND);
-        if (startResolution != TypeResolution.TYPE_RESOLVED) {
+        if (startResolution.unresolved()) {
             return startResolution;
         }
 
-        return StringFunctionUtils.resolveNumericInputType(length.dataType(), functionName());
+        return Expressions.typeMustBeNumeric(length, functionName(), ParamOrdinal.THIRD);
     }
 
     @Override
@@ -123,4 +124,4 @@ public class Substring extends ScalarFunction {
 
         return new Substring(location(), newChildren.get(0), newChildren.get(1), newChildren.get(2));
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/UnaryStringFunction.java

@@ -6,6 +6,8 @@
 package org.elasticsearch.xpack.sql.expression.function.scalar.string;
 
 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.function.scalar.UnaryScalarFunction;
 import org.elasticsearch.xpack.sql.expression.function.scalar.string.StringProcessor.StringOperation;
@@ -41,9 +43,7 @@ public abstract class UnaryStringFunction extends UnaryScalarFunction {
         if (!childrenResolved()) {
             return new TypeResolution("Unresolved children");
         }
-
+        return Expressions.typeMustBeString(field(), operation().toString(), ParamOrdinal.DEFAULT);
-        return field().dataType().isString() ? TypeResolution.TYPE_RESOLVED : new TypeResolution(
-                "'%s' requires a string type, received %s", operation(), field().dataType().esType);
     }
 
     @Override
@@ -82,4 +82,4 @@ public abstract class UnaryStringFunction extends UnaryScalarFunction {
     public int hashCode() {
         return Objects.hash(field());
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/UnaryStringIntFunction.java

@@ -6,6 +6,8 @@
 package org.elasticsearch.xpack.sql.expression.function.scalar.string;
 
 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.function.scalar.UnaryScalarFunction;
 import org.elasticsearch.xpack.sql.expression.function.scalar.string.StringProcessor.StringOperation;
@@ -43,9 +45,7 @@ public abstract class UnaryStringIntFunction extends UnaryScalarFunction {
         if (!childrenResolved()) {
             return new TypeResolution("Unresolved children");
         }
-
+        return Expressions.typeMustBeInteger(field(), operation().toString(), ParamOrdinal.DEFAULT);
-        return field().dataType().isInteger ? TypeResolution.TYPE_RESOLVED : new TypeResolution(
-                "'%s' requires a integer type, received %s", operation(), field().dataType().esType);
     }
 
     @Override
@@ -83,4 +83,4 @@ public abstract class UnaryStringIntFunction extends UnaryScalarFunction {
         UnaryStringIntFunction other = (UnaryStringIntFunction) obj;
         return Objects.equals(other.field(), field());
     }
-}
+}

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

@@ -6,8 +6,9 @@
 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.Expressions.ParamOrdinal;
 import org.elasticsearch.xpack.sql.tree.Location;
-import org.elasticsearch.xpack.sql.type.DataType;
 
 /**
  * Operator is a specialized binary predicate where both sides have the compatible types
@@ -23,7 +24,7 @@ public abstract class BinaryOperator<T, U, R, F extends PredicateBiFunction<T, U
         super(location, left, right, function);
     }
 
-    protected abstract TypeResolution resolveInputType(DataType inputType);
+    protected abstract TypeResolution resolveInputType(Expression e, Expressions.ParamOrdinal paramOrdinal);
 
     public abstract BinaryOperator<T, U, R, F> swapLeftAndRight();
 
@@ -32,14 +33,11 @@ public abstract class BinaryOperator<T, U, R, F extends PredicateBiFunction<T, U
         if (!childrenResolved()) {
             return new TypeResolution("Unresolved children");
         }
-        DataType l = left().dataType();
-        DataType r = right().dataType();
 
-        TypeResolution resolution = resolveInputType(l);
+        TypeResolution resolution = resolveInputType(left(), ParamOrdinal.FIRST);
-
+        if (resolution.unresolved()) {
-        if (resolution == TypeResolution.TYPE_RESOLVED) {
+            return resolution;
-            return resolveInputType(r);
         }
-        return resolution;
+        return resolveInputType(right(), ParamOrdinal.SECOND);
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/logical/BinaryLogic.java

@@ -25,9 +25,8 @@ public abstract class BinaryLogic extends BinaryOperator<Boolean, Boolean, Boole
     }
 
     @Override
-    protected TypeResolution resolveInputType(DataType inputType) {
+    protected TypeResolution resolveInputType(Expression e, Expressions.ParamOrdinal paramOrdinal) {
-        return DataType.BOOLEAN == inputType ? TypeResolution.TYPE_RESOLVED : new TypeResolution(
+        return Expressions.typeMustBeBoolean(e, functionName(), paramOrdinal);
-                "'%s' requires type %s not %s", symbol(), DataType.BOOLEAN.sqlName(), inputType.sqlName());
     }
 
     @Override

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/logical/Not.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.Expressions.ParamOrdinal;
 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.Scripts;
@@ -36,8 +37,7 @@ public class Not extends UnaryScalarFunction {
         if (DataType.BOOLEAN == field().dataType()) {
             return TypeResolution.TYPE_RESOLVED;
         }
-        return new TypeResolution("Cannot negate expression ([" + Expressions.name(field()) + "] of type ["
+        return Expressions.typeMustBeBoolean(field(), functionName(), ParamOrdinal.DEFAULT);
-                + field().dataType().esType + "])");
     }
 
     @Override
@@ -68,4 +68,4 @@ public class Not extends UnaryScalarFunction {
     public DataType dataType() {
         return DataType.BOOLEAN;
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/ArithmeticOperation.java

@@ -21,10 +21,8 @@ public abstract class ArithmeticOperation extends BinaryOperator<Number, Number,
     }
     
     @Override
-    protected TypeResolution resolveInputType(DataType inputType) {
+    protected TypeResolution resolveInputType(Expression e, Expressions.ParamOrdinal paramOrdinal) {
-        return inputType.isNumeric() ?
+        return Expressions.typeMustBeNumeric(e, symbol(), paramOrdinal);
-                TypeResolution.TYPE_RESOLVED :
-                new TypeResolution("'%s' requires a numeric type, received %s", symbol(), inputType.esType);
     }
 
     @Override
@@ -41,4 +39,4 @@ public abstract class ArithmeticOperation extends BinaryOperator<Number, Number,
     protected Pipe makePipe() {
         return new BinaryArithmeticPipe(location(), this, Expressions.pipe(left()), Expressions.pipe(right()), function());
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/arithmetic/Neg.java

@@ -7,6 +7,7 @@ package org.elasticsearch.xpack.sql.expression.predicate.operator.arithmetic;
 
 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.NamedExpression;
 import org.elasticsearch.xpack.sql.expression.function.scalar.UnaryScalarFunction;
 import org.elasticsearch.xpack.sql.expression.gen.processor.Processor;
@@ -38,7 +39,7 @@ public class Neg extends UnaryScalarFunction implements ScriptWeaver {
 
     @Override
     protected TypeResolution resolveType() {
-        return Expressions.typeMustBeNumeric(field());
+        return Expressions.typeMustBeNumeric(field(), functionName(), ParamOrdinal.DEFAULT);
     }
 
     @Override
@@ -65,4 +66,4 @@ public class Neg extends UnaryScalarFunction implements ScriptWeaver {
     protected Processor makeProcessor() {
         return new UnaryArithmeticProcessor(UnaryArithmeticOperation.NEGATE);
     }
-}
+}

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/BinaryComparison.java

@@ -21,7 +21,7 @@ public abstract class BinaryComparison extends BinaryOperator<Object, Object, Bo
     }
 
     @Override
-    protected TypeResolution resolveInputType(DataType inputType) {
+    protected TypeResolution resolveInputType(Expression e, Expressions.ParamOrdinal paramOrdinal) {
         return TypeResolution.TYPE_RESOLVED;
     }
 

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypes.java

@@ -8,7 +8,9 @@ package org.elasticsearch.xpack.sql.type;
 import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
 import org.joda.time.DateTime;
 
-public abstract class DataTypes {
+public final class DataTypes {
+
+    private DataTypes() {}
 
     public static boolean isNull(DataType from) {
         return from == DataType.NULL;
@@ -118,4 +120,4 @@ public abstract class DataTypes {
         // null means radix is not applicable for the given type.
         return t.isInteger ? Integer.valueOf(10) : (t.isRational ? Integer.valueOf(2) : null);
     }
-}
+}

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java

@@ -148,11 +148,6 @@ public class VerifierErrorMessagesTests extends ESTestCase {
                 verify("SELECT AVG(int) FROM test GROUP BY AVG(int)"));
     }
 
-    public void testNotSupportedAggregateOnDate() {
-        assertEquals("1:8: Argument required to be numeric ('date' type is 'date')",
-            verify("SELECT AVG(date) FROM test"));
-    }
-
     public void testGroupByOnNested() {
         assertEquals("1:38: Grouping isn't (yet) compatible with nested fields [dep.dep_id]",
                 verify("SELECT dep.dep_id FROM test GROUP BY dep.dep_id"));
@@ -237,4 +232,70 @@ public class VerifierErrorMessagesTests extends ESTestCase {
         assertEquals("1:46: expected data type [TEXT], value provided is of type [INTEGER]",
             verify("SELECT * FROM test WHERE int = 1 OR text IN (1, 2)"));
     }
+
+    public void testNotSupportedAggregateOnDate() {
+        assertEquals("1:8: [AVG] argument must be [numeric], found value [date] type [date]",
+            verify("SELECT AVG(date) FROM test"));
+    }
+
+    public void testNotSupportedAggregateOnString() {
+        assertEquals("1:8: [MAX] argument must be [numeric or date], found value [keyword] type [keyword]",
+            verify("SELECT MAX(keyword) FROM test"));
+    }
+
+    public void testInvalidTypeForStringFunction_WithOneArg() {
+        assertEquals("1:8: [LENGTH] argument must be [string], found value [1] type [integer]",
+            verify("SELECT LENGTH(1)"));
+    }
+
+    public void testInvalidTypeForNumericFunction_WithOneArg() {
+        assertEquals("1:8: [COS] argument must be [numeric], found value [foo] type [keyword]",
+            verify("SELECT COS('foo')"));
+    }
+
+    public void testInvalidTypeForBooleanFunction_WithOneArg() {
+        assertEquals("1:8: [NOT] argument must be [boolean], found value [foo] type [keyword]",
+            verify("SELECT NOT 'foo'"));
+    }
+
+    public void testInvalidTypeForStringFunction_WithTwoArgs() {
+        assertEquals("1:8: [CONCAT] first argument must be [string], found value [1] type [integer]",
+            verify("SELECT CONCAT(1, 'bar')"));
+        assertEquals("1:8: [CONCAT] second argument must be [string], found value [2] type [integer]",
+            verify("SELECT CONCAT('foo', 2)"));
+    }
+
+    public void testInvalidTypeForNumericFunction_WithTwoArgs() {
+        assertEquals("1:8: [TRUNCATE] first argument must be [numeric], found value [foo] type [keyword]",
+            verify("SELECT TRUNCATE('foo', 2)"));
+        assertEquals("1:8: [TRUNCATE] second argument must be [numeric], found value [bar] type [keyword]",
+            verify("SELECT TRUNCATE(1.2, 'bar')"));
+    }
+
+    public void testInvalidTypeForBooleanFuntion_WithTwoArgs() {
+        assertEquals("1:8: [OR] first argument must be [boolean], found value [1] type [integer]",
+            verify("SELECT 1 OR true"));
+        assertEquals("1:8: [OR] second argument must be [boolean], found value [2] type [integer]",
+            verify("SELECT true OR 2"));
+    }
+
+    public void testInvalidTypeForFunction_WithThreeArgs() {
+        assertEquals("1:8: [REPLACE] first argument must be [string], found value [1] type [integer]",
+            verify("SELECT REPLACE(1, 'foo', 'bar')"));
+        assertEquals("1:8: [REPLACE] second argument must be [string], found value [2] type [integer]",
+            verify("SELECT REPLACE('text', 2, 'bar')"));
+        assertEquals("1:8: [REPLACE] third argument must be [string], found value [3] type [integer]",
+            verify("SELECT REPLACE('text', 'foo', 3)"));
+    }
+
+    public void testInvalidTypeForFunction_WithFourArgs() {
+        assertEquals("1:8: [INSERT] first argument must be [string], found value [1] type [integer]",
+            verify("SELECT INSERT(1, 1, 2, 'new')"));
+        assertEquals("1:8: [INSERT] second argument must be [numeric], found value [foo] type [keyword]",
+            verify("SELECT INSERT('text', 'foo', 2, 'new')"));
+        assertEquals("1:8: [INSERT] third argument must be [numeric], found value [bar] type [keyword]",
+            verify("SELECT INSERT('text', 1, 'bar', 'new')"));
+        assertEquals("1:8: [INSERT] fourth argument must be [string], found value [3] type [integer]",
+            verify("SELECT INSERT('text', 1, 2, 3)"));
+    }
 }

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/predicate/InProcessorTests.java

@@ -15,6 +15,7 @@ import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.InPr
 
 import java.util.Arrays;
 
+import static org.elasticsearch.xpack.sql.expression.Literal.NULL;
 import static org.elasticsearch.xpack.sql.tree.Location.EMPTY;
 
 public class InProcessorTests extends AbstractWireSerializingTestCase<InProcessor> {
@@ -22,7 +23,6 @@ public class InProcessorTests extends AbstractWireSerializingTestCase<InProcesso
     private static final Literal ONE = L(1);
     private static final Literal TWO = L(2);
     private static final Literal THREE = L(3);
-    private static final Literal NULL = L(null);
 
     public static InProcessor randomProcessor() {
         return new InProcessor(Arrays.asList(new ConstantProcessor(randomLong()), new ConstantProcessor(randomLong())));

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/predicate/InTests.java

@@ -10,6 +10,7 @@ import org.elasticsearch.xpack.sql.expression.Literal;
 
 import java.util.Arrays;
 
+import static org.elasticsearch.xpack.sql.expression.Literal.NULL;
 import static org.elasticsearch.xpack.sql.tree.Location.EMPTY;
 
 public class InTests extends ESTestCase {
@@ -17,7 +18,6 @@ public class InTests extends ESTestCase {
     private static final Literal ONE = L(1);
     private static final Literal TWO = L(2);
     private static final Literal THREE = L(3);
-    private static final Literal NULL = L(null);
 
     public void testInWithContainedValue() {
         In in = new In(EMPTY, TWO, Arrays.asList(ONE, TWO, THREE));

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/optimizer/OptimizerTests.java

@@ -82,6 +82,7 @@ import static java.util.Arrays.asList;
 import static java.util.Collections.emptyList;
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonList;
+import static org.elasticsearch.xpack.sql.expression.Literal.NULL;
 import static org.elasticsearch.xpack.sql.tree.Location.EMPTY;
 import static org.hamcrest.Matchers.contains;
 
@@ -95,7 +96,6 @@ public class OptimizerTests extends ESTestCase {
     private static final Literal FOUR = L(4);
     private static final Literal FIVE = L(5);
     private static final Literal SIX = L(6);
-    private static final Literal NULL = L(null);
 
     public static class DummyBooleanExpression extends Expression {
 
@@ -271,6 +271,20 @@ public class OptimizerTests extends ESTestCase {
                 new ConstantFolding().rule(new Or(EMPTY, new GreaterThanOrEqual(EMPTY, TWO, THREE), Literal.TRUE)).canonical());
     }
 
+    public void testConstantFoldingBinaryLogic_WithNullHandling() {
+        assertEquals(NULL, new ConstantFolding().rule(new And(EMPTY, NULL, Literal.TRUE)).canonical());
+        assertEquals(NULL, new ConstantFolding().rule(new And(EMPTY, Literal.TRUE, NULL)).canonical());
+        assertEquals(Literal.FALSE, new ConstantFolding().rule(new And(EMPTY, NULL, Literal.FALSE)).canonical());
+        assertEquals(Literal.FALSE, new ConstantFolding().rule(new And(EMPTY, Literal.FALSE, NULL)).canonical());
+        assertEquals(NULL, new ConstantFolding().rule(new And(EMPTY, NULL, NULL)).canonical());
+
+        assertEquals(Literal.TRUE, new ConstantFolding().rule(new Or(EMPTY, NULL, Literal.TRUE)).canonical());
+        assertEquals(Literal.TRUE, new ConstantFolding().rule(new Or(EMPTY, Literal.TRUE, NULL)).canonical());
+        assertEquals(NULL, new ConstantFolding().rule(new Or(EMPTY, NULL, Literal.FALSE)).canonical());
+        assertEquals(NULL, new ConstantFolding().rule(new Or(EMPTY, Literal.FALSE, NULL)).canonical());
+        assertEquals(NULL, new ConstantFolding().rule(new Or(EMPTY, NULL, NULL)).canonical());
+    }
+
     public void testConstantFoldingRange() {
         assertEquals(true, new ConstantFolding().rule(new Range(EMPTY, FIVE, FIVE, true, L(10), false)).fold());
         assertEquals(false, new ConstantFolding().rule(new Range(EMPTY, FIVE, FIVE, false, L(10), false)).fold());