Remove TEXT output from CASE, GREATEST, LEAST

In these cases, the incoming TEXT is interpreted as KEYWORD. This has a particular effect on CASE because we now support mixing KEYWORD and TEXT in incoming arguments, and all combinations return KEYWORD. This PR also sets all expectedType values in all TestCase objects to KEYWORD if they were TEXT, so this should block any functions tests from allowing TEXT output.
craigtaverner · Oct 23, 2024 · 0fa28ad · 0fa28ad
1 parent c6ed4ef
commit 0fa28ad
Show file tree

Hide file tree

Showing 11 changed files with 127 additions and 25 deletions.
diff --git a/docs/reference/esql/functions/kibana/definition/case.json b/docs/reference/esql/functions/kibana/definition/case.json
diff --git a/docs/reference/esql/functions/kibana/definition/greatest.json b/docs/reference/esql/functions/kibana/definition/greatest.json
diff --git a/docs/reference/esql/functions/kibana/definition/least.json b/docs/reference/esql/functions/kibana/definition/least.json
diff --git a/docs/reference/esql/functions/types/case.asciidoc b/docs/reference/esql/functions/types/case.asciidoc
diff --git a/docs/reference/esql/functions/types/greatest.asciidoc b/docs/reference/esql/functions/types/greatest.asciidoc
diff --git a/docs/reference/esql/functions/types/least.asciidoc b/docs/reference/esql/functions/types/least.asciidoc
diff --git a/...c/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional/Case.java b/...c/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional/Case.java
@@ -73,7 +73,6 @@ ConditionEvaluatorSupplier toEvaluator(ToEvaluator toEvaluator) {
             "ip",
             "keyword",
             "long",
-            "text",
             "unsigned_long",
             "version" },
         description = """
@@ -195,12 +194,12 @@ protected TypeResolution resolveType() {
 
     private TypeResolution resolveValueType(Expression value, int position) {
         if (dataType == null || dataType == NULL) {
-            dataType = value.dataType();
+            dataType = value.dataType().noText();
             return TypeResolution.TYPE_RESOLVED;
         }
         return TypeResolutions.isType(
             value,
-            t -> t == dataType,
+            t -> t.noText() == dataType,
             sourceText(),
             TypeResolutions.ParamOrdinal.fromIndex(position),
             dataType.typeName()

diff --git a/...in/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional/Greatest.java b/...in/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional/Greatest.java
@@ -43,7 +43,7 @@ public class Greatest extends EsqlScalarFunction implements OptionalArgument {
     private DataType dataType;
 
     @FunctionInfo(
-        returnType = { "boolean", "date", "double", "integer", "ip", "keyword", "long", "text", "version" },
+        returnType = { "boolean", "date", "double", "integer", "ip", "keyword", "long", "version" },
         description = "Returns the maximum value from multiple columns. This is similar to <<esql-mv_max>>\n"
             + "except it is intended to run on multiple columns at once.",
         note = "When run on `keyword` or `text` fields, this returns the last string in alphabetical order. "
@@ -104,12 +104,12 @@ protected TypeResolution resolveType() {
         for (int position = 0; position < children().size(); position++) {
             Expression child = children().get(position);
             if (dataType == null || dataType == NULL) {
-                dataType = child.dataType();
+                dataType = child.dataType().noText();
                 continue;
             }
             TypeResolution resolution = TypeResolutions.isType(
                 child,
-                t -> t == dataType,
+                t -> t.noText() == dataType,
                 sourceText(),
                 TypeResolutions.ParamOrdinal.fromIndex(position),
                 dataType.typeName()

diff --git a/.../main/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional/Least.java b/.../main/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional/Least.java
@@ -43,7 +43,7 @@ public class Least extends EsqlScalarFunction implements OptionalArgument {
     private DataType dataType;
 
     @FunctionInfo(
-        returnType = { "boolean", "date", "double", "integer", "ip", "keyword", "long", "text", "version" },
+        returnType = { "boolean", "date", "double", "integer", "ip", "keyword", "long", "version" },
         description = "Returns the minimum value from multiple columns. "
             + "This is similar to <<esql-mv_min>> except it is intended to run on multiple columns at once.",
         examples = @Example(file = "math", tag = "least")
@@ -102,12 +102,12 @@ protected TypeResolution resolveType() {
         for (int position = 0; position < children().size(); position++) {
             Expression child = children().get(position);
             if (dataType == null || dataType == NULL) {
-                dataType = child.dataType();
+                dataType = child.dataType().noText();
                 continue;
             }
             TypeResolution resolution = TypeResolutions.isType(
                 child,
-                t -> t == dataType,
+                t -> t.noText() == dataType,
                 sourceText(),
                 TypeResolutions.ParamOrdinal.fromIndex(position),
                 dataType.typeName()

diff --git a/...esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/TestCaseSupplier.java b/...esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/TestCaseSupplier.java
@@ -1435,7 +1435,7 @@ public static TestCase typeError(List<TypedData> data, String expectedTypeError)
             this.source = Source.EMPTY;
             this.data = data;
             this.evaluatorToString = evaluatorToString;
-            this.expectedType = expectedType;
+            this.expectedType = expectedType == null ? null : expectedType.noText();
             @SuppressWarnings("unchecked")
             Matcher<Object> downcast = (Matcher<Object>) matcher;
             this.matcher = downcast;

diff --git a/...t/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional/CaseTests.java b/...t/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional/CaseTests.java
@@ -221,7 +221,33 @@ private static void twoAndThreeArgs(
                 )
             );
         }
-
+        if (type.noText() == DataType.KEYWORD) {
+            DataType otherType = type == DataType.KEYWORD ? DataType.TEXT : DataType.KEYWORD;
+            suppliers.add(
+                new TestCaseSupplier(
+                    "foldable " + TestCaseSupplier.nameFrom(Arrays.asList(cond, type, otherType)),
+                    List.of(DataType.BOOLEAN, type, otherType),
+                    () -> {
+                        Object lhs = randomLiteral(type).value();
+                        Object rhs = randomLiteral(otherType).value();
+                        List<TestCaseSupplier.TypedData> typedData = List.of(
+                            cond(cond, "cond").forceLiteral(),
+                            new TestCaseSupplier.TypedData(lhs, type, "lhs").forceLiteral(),
+                            new TestCaseSupplier.TypedData(rhs, otherType, "rhs")
+                        );
+                        return testCase(
+                            type.noText(),
+                            typedData,
+                            lhsOrRhs ? lhs : rhs,
+                            startsWith("LiteralsEvaluator[lit="),
+                            true,
+                            null,
+                            addBuildEvaluatorWarnings(warnings)
+                        );
+                    }
+                )
+            );
+        }
         suppliers.add(
             new TestCaseSupplier(
                 "partial foldable " + TestCaseSupplier.nameFrom(Arrays.asList(cond, type, type)),
@@ -291,6 +317,33 @@ private static void twoAndThreeArgs(
                     }
                 )
             );
+            if (type.noText() == DataType.KEYWORD) {
+                DataType otherType = type == DataType.KEYWORD ? DataType.TEXT : DataType.KEYWORD;
+                suppliers.add(
+                    new TestCaseSupplier(
+                        TestCaseSupplier.nameFrom(Arrays.asList(DataType.NULL, type, otherType)),
+                        List.of(DataType.NULL, type, otherType),
+                        () -> {
+                            Object lhs = randomLiteral(type).value();
+                            Object rhs = randomLiteral(otherType).value();
+                            List<TestCaseSupplier.TypedData> typedData = List.of(
+                                new TestCaseSupplier.TypedData(null, DataType.NULL, "cond"),
+                                new TestCaseSupplier.TypedData(lhs, type, "lhs"),
+                                new TestCaseSupplier.TypedData(rhs, otherType, "rhs")
+                            );
+                            return testCase(
+                                type.noText(),
+                                typedData,
+                                lhsOrRhs ? lhs : rhs,
+                                startsWith("CaseEagerEvaluator[conditions=[ConditionEvaluator[condition="),
+                                false,
+                                null,
+                                addWarnings(warnings)
+                            );
+                        }
+                    )
+                );
+            }
         }
         suppliers.add(
             new TestCaseSupplier(
@@ -803,7 +856,7 @@ private static String typeErrorMessage(boolean includeOrdinal, List<DataType> ty
         if (types.get(0) != DataType.BOOLEAN && types.get(0) != DataType.NULL) {
             return typeErrorMessage(includeOrdinal, types, 0, "boolean");
         }
-        DataType mainType = types.get(1);
+        DataType mainType = types.get(1).noText();
         for (int i = 2; i < types.size(); i++) {
             if (i % 2 == 0 && i != types.size() - 1) {
                 // condition
@@ -812,7 +865,7 @@ private static String typeErrorMessage(boolean includeOrdinal, List<DataType> ty
                 }
             } else {
                 // value
-                if (types.get(i) != mainType) {
+                if (types.get(i).noText() != mainType) {
                     return typeErrorMessage(includeOrdinal, types, i, mainType.typeName());
                 }
             }