diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java index ae1eb04ad82..3fb0e7721f4 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Verifier.java @@ -19,6 +19,7 @@ import org.elasticsearch.xpack.sql.expression.function.FunctionAttribute; import org.elasticsearch.xpack.sql.expression.function.Functions; import org.elasticsearch.xpack.sql.expression.function.Score; import org.elasticsearch.xpack.sql.expression.function.scalar.ScalarFunction; +import org.elasticsearch.xpack.sql.expression.predicate.conditional.ConditionalFunction; import org.elasticsearch.xpack.sql.expression.predicate.operator.comparison.In; import org.elasticsearch.xpack.sql.plan.logical.Aggregate; import org.elasticsearch.xpack.sql.plan.logical.Distinct; @@ -169,7 +170,7 @@ public final class Verifier { for (Attribute a : p.intputSet()) { String nameCandidate = useQualifier ? a.qualifiedName() : a.name(); // add only primitives (object types would only result in another error) - if (!(a.dataType() == DataType.UNSUPPORTED) && a.dataType().isPrimitive()) { + if ((a.dataType() != DataType.UNSUPPORTED) && a.dataType().isPrimitive()) { potentialMatches.add(nameCandidate); } } @@ -220,6 +221,7 @@ public final class Verifier { Set localFailures = new LinkedHashSet<>(); validateInExpression(p, localFailures); + validateConditional(p, localFailures); if (!groupingFailures.contains(p)) { checkGroupBy(p, localFailures, resolvedFunctions, groupingFailures); @@ -282,14 +284,13 @@ public final class Verifier { */ private static boolean checkGroupBy(LogicalPlan p, Set localFailures, Map resolvedFunctions, Set groupingFailures) { - return checkGroupByAgg(p, localFailures, groupingFailures, resolvedFunctions) - && checkGroupByOrder(p, localFailures, groupingFailures, resolvedFunctions) + return checkGroupByAgg(p, localFailures, resolvedFunctions) + && checkGroupByOrder(p, localFailures, groupingFailures) && checkGroupByHaving(p, localFailures, groupingFailures, resolvedFunctions); } // check whether an orderBy failed or if it occurs on a non-key - private static boolean checkGroupByOrder(LogicalPlan p, Set localFailures, - Set groupingFailures, Map functions) { + private static boolean checkGroupByOrder(LogicalPlan p, Set localFailures, Set groupingFailures) { if (p instanceof OrderBy) { OrderBy o = (OrderBy) p; LogicalPlan child = o.child(); @@ -432,8 +433,7 @@ public final class Verifier { // check whether plain columns specified in an agg are mentioned in the group-by - private static boolean checkGroupByAgg(LogicalPlan p, Set localFailures, - Set groupingFailures, Map functions) { + private static boolean checkGroupByAgg(LogicalPlan p, Set localFailures, Map functions) { if (p instanceof Aggregate) { Aggregate a = (Aggregate) p; @@ -578,7 +578,7 @@ public final class Verifier { e.forEachUp((In in) -> { DataType dt = in.value().dataType(); for (Expression value : in.list()) { - if (areTypesCompatible(in.value().dataType(), value.dataType()) == false) { + if (areTypesCompatible(dt, value.dataType()) == false) { localFailures.add(fail(value, "expected data type [%s], value provided is of type [%s]", dt, value.dataType())); return; @@ -588,6 +588,28 @@ public final class Verifier { In.class)); } + private static void validateConditional(LogicalPlan p, Set localFailures) { + p.forEachExpressions(e -> + e.forEachUp((ConditionalFunction cf) -> { + DataType dt = DataType.NULL; + + for (Expression child : cf.children()) { + if (dt == DataType.NULL) { + if (Expressions.isNull(child) == false) { + dt = child.dataType(); + } + } else { + if (areTypesCompatible(dt, child.dataType()) == false) { + localFailures.add(fail(child, "expected data type [%s], value provided is of type [%s]", + dt, child.dataType())); + return; + } + } + } + }, + ConditionalFunction.class)); + } + private static boolean areTypesCompatible(DataType left, DataType right) { if (left == right) { return true; @@ -598,4 +620,4 @@ public final class Verifier { (left.isNumeric() && right.isNumeric()); } } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java index aa6908dbed6..3d96f6ae6ae 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java @@ -11,6 +11,11 @@ import org.elasticsearch.xpack.sql.analysis.index.EsIndex; import org.elasticsearch.xpack.sql.analysis.index.IndexResolution; import org.elasticsearch.xpack.sql.analysis.index.IndexResolverTests; import org.elasticsearch.xpack.sql.expression.function.FunctionRegistry; +import org.elasticsearch.xpack.sql.expression.predicate.conditional.Coalesce; +import org.elasticsearch.xpack.sql.expression.predicate.conditional.Greatest; +import org.elasticsearch.xpack.sql.expression.predicate.conditional.IfNull; +import org.elasticsearch.xpack.sql.expression.predicate.conditional.Least; +import org.elasticsearch.xpack.sql.expression.predicate.conditional.NullIf; import org.elasticsearch.xpack.sql.parser.SqlParser; import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan; import org.elasticsearch.xpack.sql.stats.Metrics; @@ -423,4 +428,32 @@ public class VerifierErrorMessagesTests extends ESTestCase { + "[integer] in [basic], [long] in [incompatible]", incompatibleError("SELECT languages FROM \"*\" ORDER BY SIGN(ABS(emp_no))")); } -} \ No newline at end of file + + public void testConditionalWithDifferentDataTypes_SelectClause() { + @SuppressWarnings("unchecked") + String function = randomFrom(IfNull.class, NullIf.class).getSimpleName(); + assertEquals("1:" + (22 + function.length()) + + ": expected data type [INTEGER], value provided is of type [KEYWORD]", + error("SELECT 1 = 1 OR " + function + "(3, '4') > 1")); + + @SuppressWarnings("unchecked") + String arbirtraryArgsfunction = randomFrom(Coalesce.class, Greatest.class, Least.class).getSimpleName(); + assertEquals("1:" + (34 + arbirtraryArgsfunction.length()) + + ": expected data type [INTEGER], value provided is of type [KEYWORD]", + error("SELECT 1 = 1 OR " + arbirtraryArgsfunction + "(null, null, 3, '4') > 1")); + } + + public void testConditionalWithDifferentDataTypes_WhereClause() { + @SuppressWarnings("unchecked") + String function = randomFrom(IfNull.class, NullIf.class).getSimpleName(); + assertEquals("1:" + (34 + function.length()) + + ": expected data type [KEYWORD], value provided is of type [INTEGER]", + error("SELECT * FROM test WHERE " + function + "('foo', 4) > 1")); + + @SuppressWarnings("unchecked") + String arbirtraryArgsfunction = randomFrom(Coalesce.class, Greatest.class, Least.class).getSimpleName(); + assertEquals("1:" + (46 + arbirtraryArgsfunction.length()) + + ": expected data type [KEYWORD], value provided is of type [INTEGER]", + error("SELECT * FROM test WHERE " + arbirtraryArgsfunction + "(null, null, 'foo', 4) > 1")); + } +}