SQL: Fix issue with IN not resolving to underlying keyword field (#38440)

- Add resolution to the exact keyword field (if exists) for text fields.
- Add proper verification and error message if underlying keyword
doesn'texist.
- Move check for field attribute in the comparison list to the
`resolveType()` method of `IN`.

Fixes: #38424

Author: matriv

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

@@ -5,8 +5,10 @@
  */
 package org.elasticsearch.xpack.sql.expression.predicate.operator.comparison;
 
+import org.elasticsearch.xpack.sql.analysis.index.MappingException;
 import org.elasticsearch.xpack.sql.expression.Expression;
 import org.elasticsearch.xpack.sql.expression.Expressions;
+import org.elasticsearch.xpack.sql.expression.FieldAttribute;
 import org.elasticsearch.xpack.sql.expression.Foldables;
 import org.elasticsearch.xpack.sql.expression.Nullability;
 import org.elasticsearch.xpack.sql.expression.function.scalar.ScalarFunction;
@@ -25,6 +27,7 @@
 import java.util.StringJoiner;
 import java.util.stream.Collectors;
 
+import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
 import static org.elasticsearch.xpack.sql.expression.gen.script.ParamsBuilder.paramsBuilder;
 
 public class In extends ScalarFunction {
@@ -113,6 +116,27 @@ protected Pipe makePipe() {
         return new InPipe(location(), this, children().stream().map(Expressions::pipe).collect(Collectors.toList()));
     }
 
+    @Override
+    protected TypeResolution resolveType() {
+        if (value instanceof FieldAttribute) {
+            try {
+                ((FieldAttribute) value).exactAttribute();
+            } catch (MappingException e) {
+                return new TypeResolution(format(null, "[{}] cannot operate on field of data type [{}]: {}",
+                    functionName(), value().dataType().esType, e.getMessage()));
+            }
+        }
+
+        for (Expression ex : list) {
+            if (ex.foldable() == false) {
+                return new TypeResolution(format(null, "Comparisons against variables are not (currently) supported; offender [{}] in [{}]",
+                    Expressions.name(ex),
+                    name()));
+            }
+        }
+        return super.resolveType();
+    }
+
     @Override
     public int hashCode() {
         return Objects.hash(value, list);

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java

@@ -100,7 +100,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
-import java.util.Optional;
 import java.util.function.Supplier;
 
 import static java.util.Collections.singletonList;
@@ -682,16 +681,6 @@ static class InComparisons extends ExpressionTranslator<In> {
 
         @Override
         protected QueryTranslation asQuery(In in, boolean onAggs) {
-            Optional<Expression> firstNotFoldable = in.list().stream().filter(expression -> !expression.foldable()).findFirst();
-
-            if (firstNotFoldable.isPresent()) {
-                throw new SqlIllegalArgumentException(
-                    "Line {}:{}: Comparisons against variables are not (currently) supported; offender [{}] in [{}]",
-                    firstNotFoldable.get().location().getLineNumber(),
-                    firstNotFoldable.get().location().getColumnNumber(),
-                    Expressions.name(firstNotFoldable.get()),
-                    in.name());
-            }
 
             if (in.value() instanceof NamedExpression) {
                 NamedExpression ne = (NamedExpression) in.value();
@@ -709,7 +698,9 @@ protected QueryTranslation asQuery(In in, boolean onAggs) {
                 else {
                     Query q = null;
                     if (in.value() instanceof FieldAttribute) {
-                        q = new TermsQuery(in.location(), ne.name(), in.list());
+                        FieldAttribute fa = (FieldAttribute) in.value();
+                        // equality should always be against an exact match (which is important for strings)
+                        q = new TermsQuery(in.location(), fa.isInexact() ? fa.exactAttribute().name() : fa.name(), in.list());
                     } else {
                         q = new ScriptQuery(in.location(), in.asScript());
                     }

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

@@ -351,23 +351,34 @@ public void testInNestedWithDifferentDataTypesFromLeftValue_SelectClause() {
     }
 
     public void testInWithDifferentDataTypes_WhereClause() {
-        assertEquals("1:49: expected data type [TEXT], value provided is of type [INTEGER]",
-            error("SELECT * FROM test WHERE text IN ('foo', 'bar', 4)"));
+        assertEquals("1:52: expected data type [KEYWORD], value provided is of type [INTEGER]",
+            error("SELECT * FROM test WHERE keyword IN ('foo', 'bar', 4)"));
     }
 
     public void testInNestedWithDifferentDataTypes_WhereClause() {
-        assertEquals("1:60: expected data type [TEXT], value provided is of type [INTEGER]",
-            error("SELECT * FROM test WHERE int = 1 OR text IN ('foo', 'bar', 2)"));
+        assertEquals("1:63: expected data type [KEYWORD], value provided is of type [INTEGER]",
+            error("SELECT * FROM test WHERE int = 1 OR keyword IN ('foo', 'bar', 2)"));
     }
 
     public void testInWithDifferentDataTypesFromLeftValue_WhereClause() {
-        assertEquals("1:35: expected data type [TEXT], value provided is of type [INTEGER]",
-            error("SELECT * FROM test WHERE text IN (1, 2)"));
+        assertEquals("1:38: expected data type [KEYWORD], value provided is of type [INTEGER]",
+            error("SELECT * FROM test WHERE keyword IN (1, 2)"));
     }
 
     public void testInNestedWithDifferentDataTypesFromLeftValue_WhereClause() {
-        assertEquals("1:46: expected data type [TEXT], value provided is of type [INTEGER]",
-            error("SELECT * FROM test WHERE int = 1 OR text IN (1, 2)"));
+        assertEquals("1:49: expected data type [KEYWORD], value provided is of type [INTEGER]",
+            error("SELECT * FROM test WHERE int = 1 OR keyword IN (1, 2)"));
+    }
+
+    public void testInWithFieldInListOfValues() {
+        assertEquals("1:30: Comparisons against variables are not (currently) supported; offender [int] in [int IN (1, int)]",
+            error("SELECT * FROM test WHERE int IN (1, int)"));
+    }
+
+    public void testInOnFieldTextWithNoKeyword() {
+        assertEquals("1:31: [IN] cannot operate on field of data type [text]: " +
+            "No keyword/multi-field defined exact matches for [text]; define one or use MATCH/QUERY instead",
+            error("SELECT * FROM test WHERE text IN ('foo', 'bar')"));
     }
 
     public void testNotSupportedAggregateOnDate() {

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java

@@ -307,8 +307,8 @@ public void testTranslateInExpression_WhereClause() {
             tq.asBuilder().toString().replaceAll("\\s", ""));
     }
 
-    public void testTranslateInExpression_WhereClauseAndNullHandling() {
-        LogicalPlan p = plan("SELECT * FROM test WHERE keyword IN ('foo', null, 'lala', null, 'foo', concat('la', 'la'))");
+    public void testTranslateInExpression_WhereClause_TextFieldWithKeyword() {
+        LogicalPlan p = plan("SELECT * FROM test WHERE some.string IN ('foo', 'bar', 'lala', 'foo', concat('la', 'la'))");
         assertTrue(p instanceof Project);
         assertTrue(p.children().get(0) instanceof Filter);
         Expression condition = ((Filter) p.children().get(0)).condition();
@@ -317,19 +317,22 @@ public void testTranslateInExpression_WhereClauseAndNullHandling() {
         Query query = translation.query;
         assertTrue(query instanceof TermsQuery);
         TermsQuery tq = (TermsQuery) query;
-        assertEquals("{\"terms\":{\"keyword\":[\"foo\",\"lala\"],\"boost\":1.0}}",
+        assertEquals("{\"terms\":{\"some.string.typical\":[\"foo\",\"bar\",\"lala\"],\"boost\":1.0}}",
             tq.asBuilder().toString().replaceAll("\\s", ""));
     }
 
-    public void testTranslateInExpressionInvalidValues_WhereClause() {
-        LogicalPlan p = plan("SELECT * FROM test WHERE keyword IN ('foo', 'bar', keyword)");
+    public void testTranslateInExpression_WhereClauseAndNullHandling() {
+        LogicalPlan p = plan("SELECT * FROM test WHERE keyword IN ('foo', null, 'lala', null, 'foo', concat('la', 'la'))");
         assertTrue(p instanceof Project);
         assertTrue(p.children().get(0) instanceof Filter);
         Expression condition = ((Filter) p.children().get(0)).condition();
         assertFalse(condition.foldable());
-        SqlIllegalArgumentException ex = expectThrows(SqlIllegalArgumentException.class, () -> QueryTranslator.toQuery(condition, false));
-        assertEquals("Line 1:52: Comparisons against variables are not (currently) supported; " +
-                "offender [keyword] in [keyword IN (foo, bar, keyword)]", ex.getMessage());
+        QueryTranslation translation = QueryTranslator.toQuery(condition, false);
+        Query query = translation.query;
+        assertTrue(query instanceof TermsQuery);
+        TermsQuery tq = (TermsQuery) query;
+        assertEquals("{\"terms\":{\"keyword\":[\"foo\",\"lala\"],\"boost\":1.0}}",
+            tq.asBuilder().toString().replaceAll("\\s", ""));
     }
 
     public void testTranslateInExpression_WhereClause_Painless() {