SQL: Verify filter's condition type (elastic#66268)

* Verify filter's condition type

This adds a check in the verifier to check if filter's condition is of a
boolean type and fail the request otherwise.

(cherry picked from commit 3aec1a3)

x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/analysis/Verifier.java

@@ -51,6 +51,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;
 
 /**
@@ -148,6 +149,7 @@ Collection<Failure> verify(LogicalPlan plan) {
                 Set<Failure> localFailures = new LinkedHashSet<>();
                 failures.addAll(localFailures);
 
+                checkFilterConditionType(p, localFailures);
                 // mark the plan as analyzed
                 // if everything checks out
                 if (failures.isEmpty()) {
@@ -236,4 +238,4 @@ Collection<Failure> verify(LogicalPlan plan) {
 
         return failures;
     }
-}
+}

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"));
     }

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<Failure> 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()));
+            }
+        }
+    }
+
+}

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;
 

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<Failure> verify(LogicalPlan plan) {
                     return;
                 }
 
+                checkFilterConditionType(p, localFailures);
                 checkGroupingFunctionInGroupBy(p, localFailures);
                 checkFilterOnAggs(p, localFailures, attributeRefs);
                 checkFilterOnGrouping(p, localFailures, attributeRefs);

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<String, List<String>> 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"));
     }