diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/analysis/Verifier.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/analysis/Verifier.java index 6a361c5f161d0..7112ef3e5a563 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/analysis/Verifier.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/analysis/Verifier.java @@ -53,6 +53,7 @@ import static org.elasticsearch.xpack.eql.stats.FeatureMetric.SEQUENCE_QUERIES_THREE; import static org.elasticsearch.xpack.eql.stats.FeatureMetric.SEQUENCE_QUERIES_TWO; import static org.elasticsearch.xpack.eql.stats.FeatureMetric.SEQUENCE_UNTIL; +import static org.elasticsearch.xpack.ql.analyzer.VerifierChecks.checkFilterConditionType; import static org.elasticsearch.xpack.ql.common.Failure.fail; /** @@ -153,6 +154,7 @@ Collection verify(LogicalPlan plan) { return; } + checkFilterConditionType(p, localFailures); checkJoinKeyTypes(p, localFailures); // mark the plan as analyzed // if everything checks out diff --git a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/VerifierTests.java b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/VerifierTests.java index e2722f1f1e403..ac9e72a1f997f 100644 --- a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/VerifierTests.java +++ b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/VerifierTests.java @@ -71,6 +71,16 @@ public void testBasicQuery() { accept("foo where true"); } + public void testQueryCondition() { + accept("any where bool"); + assertEquals("1:11: Condition expression needs to be boolean, found [LONG]", error("any where pid")); + assertEquals("1:11: Condition expression needs to be boolean, found [DATETIME]", error("any where @timestamp")); + assertEquals("1:11: Condition expression needs to be boolean, found [KEYWORD]", error("any where command_line")); + assertEquals("1:11: Condition expression needs to be boolean, found [TEXT]", error("any where hostname")); + assertEquals("1:11: Condition expression needs to be boolean, found [KEYWORD]", error("any where constant_keyword")); + assertEquals("1:11: Condition expression needs to be boolean, found [IP]", error("any where source_address")); + } + public void testQueryStartsWithNumber() { assertEquals("1:1: no viable alternative at input '42'", errorParsing("42 where true")); } diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/analyzer/VerifierChecks.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/analyzer/VerifierChecks.java new file mode 100644 index 0000000000000..ac9cd133f2a46 --- /dev/null +++ b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/analyzer/VerifierChecks.java @@ -0,0 +1,30 @@ +/* + * 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.ql.analyzer; + +import org.elasticsearch.xpack.ql.common.Failure; +import org.elasticsearch.xpack.ql.expression.Expression; +import org.elasticsearch.xpack.ql.plan.logical.Filter; +import org.elasticsearch.xpack.ql.plan.logical.LogicalPlan; + +import java.util.Set; + +import static org.elasticsearch.xpack.ql.common.Failure.fail; +import static org.elasticsearch.xpack.ql.type.DataTypes.BOOLEAN; + +public final class VerifierChecks { + + public static void checkFilterConditionType(LogicalPlan p, Set localFailures) { + if (p instanceof Filter) { + Expression condition = ((Filter) p).condition(); + if (condition.dataType() != BOOLEAN) { + localFailures.add(fail(condition, "Condition expression needs to be boolean, found [{}]", condition.dataType())); + } + } + } + +} diff --git a/x-pack/plugin/sql/qa/server/src/main/resources/conditionals.csv-spec b/x-pack/plugin/sql/qa/server/src/main/resources/conditionals.csv-spec index bf72f9958cb97..d0b764e7a4fcb 100644 --- a/x-pack/plugin/sql/qa/server/src/main/resources/conditionals.csv-spec +++ b/x-pack/plugin/sql/qa/server/src/main/resources/conditionals.csv-spec @@ -326,6 +326,23 @@ Pettey Heyers ; +iifConditionWhere +SELECT last_name FROM test_emp WHERE IIF(LENGTH(last_name) < 7, true, false) LIMIT 10; + + last_name +----------- +Simmel +Peac +Sluis +Terkki +Genin +Peha +Erde +Famili +Pettey +Heyers +; + iifOrderBy SELECT last_name FROM test_emp ORDER BY IIF(languages >= 3, 'first', 'second'), emp_no LIMIT 10; 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 4d58f0db1f4b3..4623fc4083d9b 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 @@ -69,6 +69,7 @@ import java.util.function.Consumer; import static java.util.stream.Collectors.toMap; +import static org.elasticsearch.xpack.ql.analyzer.VerifierChecks.checkFilterConditionType; import static org.elasticsearch.xpack.ql.common.Failure.fail; import static org.elasticsearch.xpack.ql.util.CollectionUtils.combine; import static org.elasticsearch.xpack.sql.stats.FeatureMetric.COMMAND; @@ -208,6 +209,7 @@ Collection verify(LogicalPlan plan) { return; } + checkFilterConditionType(p, localFailures); checkGroupingFunctionInGroupBy(p, localFailures); checkFilterOnAggs(p, localFailures, attributeRefs); checkFilterOnGrouping(p, localFailures, attributeRefs); 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 83e8d52d15f87..2aeb316da01b8 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 @@ -26,7 +26,9 @@ import org.elasticsearch.xpack.sql.stats.Metrics; import java.util.Arrays; +import java.util.HashMap; import java.util.LinkedHashMap; +import java.util.List; import java.util.Locale; import java.util.Map; @@ -95,6 +97,19 @@ public void testMissingIndex() { assertEquals("1:17: Unknown index [missing]", error(IndexResolution.notFound("missing"), "SELECT foo FROM missing")); } + public void testNonBooleanFilter() { + Map> testData = new HashMap<>(); + testData.put("INTEGER", List.of("int", "int + 1", "ABS(int)", "ASCII(keyword)")); + testData.put("KEYWORD", List.of("keyword", "RTRIM(keyword)", "IIF(true, 'true', 'false')")); + testData.put("DATETIME", List.of("date", "date + INTERVAL 1 DAY", "NOW()")); + for (String typeName : testData.keySet()) { + for (String exp : testData.get(typeName)) { + assertEquals("1:26: Condition expression needs to be boolean, found [" + typeName + "]", + error("SELECT * FROM test WHERE " + exp)); + } + } + } + public void testMissingColumn() { assertEquals("1:8: Unknown column [xxx]", error("SELECT xxx FROM test")); } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java index 454548774940b..ec7e3992647db 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java @@ -2508,15 +2508,4 @@ public void testAddMissingEqualsToNestedBoolField() { assertEquals(expectedCondition, condition); } - - public void testNotAddMissingEqualsToNonBoolField() { - LogicalPlan p = plan("SELECT bool FROM test WHERE " + randomFrom("int", "text", "keyword", "date")); - assertTrue(p instanceof Project); - - p = ((Project) p).child(); - assertTrue(p instanceof Filter); - - Expression condition = ((Filter) p).condition(); - assertTrue(condition instanceof FieldAttribute); - } }