Better DataType string checks (elastic#114863)

ioanatia · georgewallace · commit 85f957ecd57d · 2024-10-24T20:40:46.000-06:00
* Use DataType.isString

* Add DataType.stringTypes()

* Fix shouldHideSignature check
diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataType.java
@@ -266,6 +266,8 @@ public enum DataType {
         .sorted(Comparator.comparing(DataType::typeName))
         .toList();
 
+    private static final Collection<DataType> STRING_TYPES = DataType.types().stream().filter(DataType::isString).toList();
+
     private static final Map<String, DataType> NAME_TO_TYPE = TYPES.stream().collect(toUnmodifiableMap(DataType::typeName, t -> t));
 
     private static final Map<String, DataType> ES_TO_TYPE;
@@ -292,6 +294,10 @@ public static Collection<DataType> types() {
         return TYPES;
     }
 
+    public static Collection<DataType> stringTypes() {
+        return STRING_TYPES;
+    }
+
     /**
      * Resolve a type from a name. This name is sometimes user supplied,
      * like in the case of {@code ::<typename>} and is sometimes the name
diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataTypeConverter.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/type/DataTypeConverter.java
@@ -30,11 +30,9 @@
 import static org.elasticsearch.xpack.esql.core.type.DataType.FLOAT;
 import static org.elasticsearch.xpack.esql.core.type.DataType.INTEGER;
 import static org.elasticsearch.xpack.esql.core.type.DataType.IP;
-import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD;
 import static org.elasticsearch.xpack.esql.core.type.DataType.LONG;
 import static org.elasticsearch.xpack.esql.core.type.DataType.NULL;
 import static org.elasticsearch.xpack.esql.core.type.DataType.SHORT;
-import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT;
 import static org.elasticsearch.xpack.esql.core.type.DataType.UNSIGNED_LONG;
 import static org.elasticsearch.xpack.esql.core.type.DataType.VERSION;
 import static org.elasticsearch.xpack.esql.core.type.DataType.isDateTime;
@@ -62,7 +60,7 @@ public static Converter converterFor(DataType from, DataType to) {
             return DefaultConverter.TO_NULL;
         }
         // proper converters
-        if (to == KEYWORD || to == TEXT) {
+        if (isString(to)) {
             return conversionToString(from);
         }
         if (to == LONG) {
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/CountDistinct.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/CountDistinct.java
@@ -209,7 +209,7 @@ public AggregatorFunctionSupplier supplier(List<Integer> inputChannels) {
         if (type == DataType.DOUBLE) {
             return new CountDistinctDoubleAggregatorFunctionSupplier(inputChannels, precision);
         }
-        if (type == DataType.KEYWORD || type == DataType.IP || type == DataType.VERSION || type == DataType.TEXT) {
+        if (DataType.isString(type) || type == DataType.IP || type == DataType.VERSION) {
             return new CountDistinctBytesRefAggregatorFunctionSupplier(inputChannels, precision);
         }
         throw EsqlIllegalArgumentException.illegalDataType(type);
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Max.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Max.java
@@ -128,7 +128,7 @@ public final AggregatorFunctionSupplier supplier(List<Integer> inputChannels) {
         if (type == DataType.IP) {
             return new MaxIpAggregatorFunctionSupplier(inputChannels);
         }
-        if (type == DataType.VERSION || type == DataType.KEYWORD || type == DataType.TEXT) {
+        if (type == DataType.VERSION || DataType.isString(type)) {
             return new MaxBytesRefAggregatorFunctionSupplier(inputChannels);
         }
         throw EsqlIllegalArgumentException.illegalDataType(type);
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Min.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Min.java
@@ -128,7 +128,7 @@ public final AggregatorFunctionSupplier supplier(List<Integer> inputChannels) {
         if (type == DataType.IP) {
             return new MinIpAggregatorFunctionSupplier(inputChannels);
         }
-        if (type == DataType.VERSION || type == DataType.KEYWORD || type == DataType.TEXT) {
+        if (type == DataType.VERSION || DataType.isString(type)) {
             return new MinBytesRefAggregatorFunctionSupplier(inputChannels);
         }
         throw EsqlIllegalArgumentException.illegalDataType(type);
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional/Greatest.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional/Greatest.java
@@ -155,11 +155,7 @@ public ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {
         if (dataType == DataType.LONG || dataType == DataType.DATETIME) {
             return new GreatestLongEvaluator.Factory(source(), factories);
         }
-        if (dataType == DataType.KEYWORD
-            || dataType == DataType.TEXT
-            || dataType == DataType.IP
-            || dataType == DataType.VERSION
-            || dataType == DataType.UNSUPPORTED) {
+        if (DataType.isString(dataType) || dataType == DataType.IP || dataType == DataType.VERSION || dataType == DataType.UNSUPPORTED) {
 
             return new GreatestBytesRefEvaluator.Factory(source(), factories);
         }
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional/Least.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/conditional/Least.java
@@ -154,11 +154,7 @@ public ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {
         if (dataType == DataType.LONG || dataType == DataType.DATETIME) {
             return new LeastLongEvaluator.Factory(source(), factories);
         }
-        if (dataType == DataType.KEYWORD
-            || dataType == DataType.TEXT
-            || dataType == DataType.IP
-            || dataType == DataType.VERSION
-            || dataType == DataType.UNSUPPORTED) {
+        if (DataType.isString(dataType) || dataType == DataType.IP || dataType == DataType.VERSION || dataType == DataType.UNSUPPORTED) {
 
             return new LeastBytesRefEvaluator.Factory(source(), factories);
         }
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/AbstractConvertFunction.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/AbstractConvertFunction.java
@@ -48,7 +48,6 @@ public abstract class AbstractConvertFunction extends UnaryScalarFunction {
 
     // the numeric types convert functions need to handle; the other numeric types are converted upstream to one of these
     private static final List<DataType> NUMERIC_TYPES = List.of(DataType.INTEGER, DataType.LONG, DataType.UNSIGNED_LONG, DataType.DOUBLE);
-    public static final List<DataType> STRING_TYPES = DataType.types().stream().filter(DataType::isString).toList();
 
     protected AbstractConvertFunction(Source source, Expression field) {
         super(source, field);
@@ -90,9 +89,9 @@ private static String supportedTypesNames(Set<DataType> types) {
             NUMERIC_TYPES.forEach(supportTypes::remove);
         }
 
-        if (types.containsAll(STRING_TYPES)) {
+        if (types.containsAll(DataType.stringTypes())) {
             supportedTypesNames.add("string");
-            STRING_TYPES.forEach(supportTypes::remove);
+            DataType.stringTypes().forEach(supportTypes::remove);
         }
 
         supportTypes.forEach(t -> supportedTypesNames.add(t.nameUpper().toLowerCase(Locale.ROOT)));
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InsensitiveEqualsMapper.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InsensitiveEqualsMapper.java
@@ -34,7 +34,7 @@ public final ExpressionEvaluator.Factory map(InsensitiveEquals bc, Layout layout
 
         var leftEval = toEvaluator(bc.left(), layout);
         var rightEval = toEvaluator(bc.right(), layout);
-        if (leftType == DataType.KEYWORD || leftType == DataType.TEXT) {
+        if (DataType.isString(leftType)) {
             if (bc.right().foldable() && DataType.isString(rightType)) {
                 BytesRef rightVal = BytesRefs.toBytesRef(bc.right().fold());
                 Automaton automaton = InsensitiveEquals.automaton(rightVal);
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/AbstractFunctionTestCase.java
@@ -1283,13 +1283,6 @@ public void allMemoryReleased() {
         }
     }
 
-    /**
-     * All string types (keyword, text, match_only_text, etc).
-     */
-    protected static DataType[] strings() {
-        return DataType.types().stream().filter(DataType::isString).toArray(DataType[]::new);
-    }
-
     /**
      * Validate that we know the types for all the test cases already created
      * @param suppliers - list of suppliers before adding in the illegal type combinations
@@ -1316,10 +1309,9 @@ private static boolean isAggregation() {
      */
     private static boolean shouldHideSignature(List<DataType> argTypes, DataType returnType) {
         for (DataType dt : DataType.UNDER_CONSTRUCTION.keySet()) {
-            if (returnType == dt) {
+            if (returnType == dt || argTypes.contains(dt)) {
                 return true;
             }
-            return argTypes.contains(dt);
         }
         return false;
     }
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/TestCaseSupplier.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/TestCaseSupplier.java
@@ -21,7 +21,6 @@
 import org.elasticsearch.xpack.esql.core.tree.Source;
 import org.elasticsearch.xpack.esql.core.type.DataType;
 import org.elasticsearch.xpack.esql.core.util.NumericUtils;
-import org.elasticsearch.xpack.esql.expression.function.scalar.convert.AbstractConvertFunction;
 import org.elasticsearch.xpack.versionfield.Version;
 import org.hamcrest.Matcher;
 
@@ -91,7 +90,7 @@ public static List<TestCaseSupplier> stringCases(
         List<TypedDataSupplier> lhsSuppliers = new ArrayList<>();
         List<TypedDataSupplier> rhsSuppliers = new ArrayList<>();
         List<TestCaseSupplier> suppliers = new ArrayList<>();
-        for (DataType type : AbstractConvertFunction.STRING_TYPES) {
+        for (DataType type : DataType.stringTypes()) {
             lhsSuppliers.addAll(stringCases(type));
             rhsSuppliers.addAll(stringCases(type));
             casesCrossProduct(
@@ -760,7 +759,7 @@ public static void forUnaryStrings(
         Function<BytesRef, Object> expectedValue,
         Function<BytesRef, List<String>> expectedWarnings
     ) {
-        for (DataType type : AbstractConvertFunction.STRING_TYPES) {
+        for (DataType type : DataType.stringTypes()) {
             unary(
                 suppliers,
                 expectedEvaluatorToString,
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/fulltext/MatchTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/fulltext/MatchTests.java
@@ -20,7 +20,6 @@
 import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
 import org.hamcrest.Matcher;
 
-import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Set;
@@ -40,8 +39,8 @@ public static Iterable<Object[]> parameters() {
         Set<DataType> supported = Set.of(DataType.KEYWORD, DataType.TEXT);
         List<Set<DataType>> supportedPerPosition = List.of(supported, supported);
         List<TestCaseSupplier> suppliers = new LinkedList<>();
-        for (DataType fieldType : validStringDataTypes()) {
-            for (DataType queryType : validStringDataTypes()) {
+        for (DataType fieldType : DataType.stringTypes()) {
+            for (DataType queryType : DataType.stringTypes()) {
                 suppliers.add(
                     new TestCaseSupplier(
                         "<" + fieldType + "-ES field, " + queryType + ">",
@@ -67,10 +66,6 @@ private static String matchTypeErrorSupplier(boolean includeOrdinal, List<Set<Da
         return "[] cannot operate on [" + types.getFirst().typeName() + "], which is not a field from an index mapping";
     }
 
-    private static List<DataType> validStringDataTypes() {
-        return Arrays.stream(DataType.values()).filter(DataType::isString).toList();
-    }
-
     private static TestCaseSupplier.TestCase testCase(
         DataType fieldType,
         String field,
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/fulltext/QueryStringTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/fulltext/QueryStringTests.java
@@ -19,7 +19,6 @@
 import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
 import org.hamcrest.Matcher;
 
-import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.function.Supplier;
@@ -36,7 +35,7 @@ public QueryStringTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> te
     @ParametersFactory
     public static Iterable<Object[]> parameters() {
         List<TestCaseSupplier> suppliers = new LinkedList<>();
-        for (DataType strType : Arrays.stream(DataType.values()).filter(DataType::isString).toList()) {
+        for (DataType strType : DataType.stringTypes()) {
             suppliers.add(
                 new TestCaseSupplier(
                     "<" + strType + ">",
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToVersionTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToVersionTests.java
@@ -49,7 +49,7 @@ public static Iterable<Object[]> parameters() {
         );
 
         // But strings that are shaped like versions do parse to valid versions
-        for (DataType inputType : AbstractConvertFunction.STRING_TYPES) {
+        for (DataType inputType : DataType.stringTypes()) {
             TestCaseSupplier.unary(
                 suppliers,
                 read,
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/AbstractTrimTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/AbstractTrimTests.java
@@ -21,7 +21,7 @@
 public abstract class AbstractTrimTests extends AbstractScalarFunctionTestCase {
     static Iterable<Object[]> parameters(String name, boolean trimLeading, boolean trimTrailing) {
         List<TestCaseSupplier> suppliers = new ArrayList<>();
-        for (DataType type : strings()) {
+        for (DataType type : DataType.stringTypes()) {
             suppliers.add(new TestCaseSupplier("no whitespace/" + type, List.of(type), () -> {
                 String text = randomAlphaOfLength(8);
                 return testCase(name, type, text, text);
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/ConcatTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/ConcatTests.java
@@ -58,9 +58,7 @@ public static Iterable<Object[]> parameters() {
                 if (rhs == DataType.NULL || DataType.isRepresentable(rhs) == false) {
                     continue;
                 }
-                boolean lhsIsString = lhs == DataType.KEYWORD || lhs == DataType.TEXT;
-                boolean rhsIsString = rhs == DataType.KEYWORD || rhs == DataType.TEXT;
-                if (lhsIsString && rhsIsString) {
+                if (DataType.isString(lhs) && DataType.isString(rhs)) {
                     continue;
                 }
 
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/EndsWithTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/EndsWithTests.java
@@ -18,7 +18,6 @@
 import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
 import org.hamcrest.Matcher;
 
-import java.util.Arrays;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.function.Supplier;
@@ -33,8 +32,8 @@ public EndsWithTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> testC
     @ParametersFactory
     public static Iterable<Object[]> parameters() {
         List<TestCaseSupplier> suppliers = new LinkedList<>();
-        for (DataType strType : Arrays.stream(DataType.values()).filter(DataType::isString).toList()) {
-            for (DataType suffixType : Arrays.stream(DataType.values()).filter(DataType::isString).toList()) {
+        for (DataType strType : DataType.stringTypes()) {
+            for (DataType suffixType : DataType.stringTypes()) {
                 suppliers.add(
                     new TestCaseSupplier(
                         "<" + strType + ">, empty <" + suffixType + ">",
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/LocateTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/LocateTests.java
@@ -35,13 +35,11 @@ public LocateTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> testCas
         this.testCase = testCaseSupplier.get();
     }
 
-    private static final DataType[] STRING_TYPES = new DataType[] { DataType.KEYWORD, DataType.TEXT };
-
     @ParametersFactory
     public static Iterable<Object[]> parameters() {
         List<TestCaseSupplier> suppliers = new ArrayList<>();
-        for (DataType strType : STRING_TYPES) {
-            for (DataType substrType : STRING_TYPES) {
+        for (DataType strType : DataType.stringTypes()) {
+            for (DataType substrType : DataType.stringTypes()) {
                 suppliers.add(
                     supplier(
                         "",
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/RLikeTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/RLikeTests.java
@@ -69,7 +69,7 @@ static Iterable<Object[]> parameters(Function<String, String> escapeString, Supp
         casesForString(cases, "6 bytes, 2 code points", () -> "❗️", false, escapeString, optionalPattern);
         casesForString(cases, "100 random code points", () -> randomUnicodeOfCodepointLength(100), true, escapeString, optionalPattern);
         for (DataType type : DataType.types()) {
-            if (type == DataType.KEYWORD || type == DataType.TEXT || type == DataType.NULL) {
+            if (DataType.isString(type) || type == DataType.NULL) {
                 continue;
             }
             if (DataType.isRepresentable(type) == false) {
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/StartsWithTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/StartsWithTests.java
@@ -18,7 +18,6 @@
 import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.List;
 import java.util.function.Supplier;
 
@@ -32,8 +31,8 @@ public StartsWithTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> tes
     @ParametersFactory
     public static Iterable<Object[]> parameters() {
         List<TestCaseSupplier> suppliers = new ArrayList<>();
-        for (DataType strType : Arrays.stream(DataType.values()).filter(DataType::isString).toList()) {
-            for (DataType prefixType : Arrays.stream(DataType.values()).filter(DataType::isString).toList()) {
+        for (DataType strType : DataType.stringTypes()) {
+            for (DataType prefixType : DataType.stringTypes()) {
                 suppliers.add(new TestCaseSupplier(List.of(strType, prefixType), () -> {
                     String str = randomAlphaOfLength(5);
                     String prefix = randomAlphaOfLength(5);
diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/type/EsqlDataTypeConverterTests.java
@@ -62,8 +62,7 @@ public void testCommonTypeNull() {
     }
 
     public void testCommonTypeStrings() {
-        List<DataType> STRINGS = Arrays.stream(DataType.values()).filter(DataType::isString).toList();
-        for (DataType dataType1 : STRINGS) {
+        for (DataType dataType1 : DataType.stringTypes()) {
             for (DataType dataType2 : DataType.values()) {
                 if (dataType2 == NULL) {
                     assertEqualsCommonType(dataType1, NULL, dataType1);

Original file line number	Diff line number	Diff line change
`@@ -209,7 +209,7 @@ public AggregatorFunctionSupplier supplier(List<Integer> inputChannels) {`
`209`	`209`	`if (type == DataType.DOUBLE) {`
`210`	`210`	`return new CountDistinctDoubleAggregatorFunctionSupplier(inputChannels, precision);`
`211`	`211`	`}`
`212`		`- if (type == DataType.KEYWORD \|\| type == DataType.IP \|\| type == DataType.VERSION \|\| type == DataType.TEXT) {`
	`212`	`+ if (DataType.isString(type) \|\| type == DataType.IP \|\| type == DataType.VERSION) {`
`213`	`213`	`return new CountDistinctBytesRefAggregatorFunctionSupplier(inputChannels, precision);`
`214`	`214`	`}`
`215`	`215`	`throw EsqlIllegalArgumentException.illegalDataType(type);`
Original file line number	Diff line number	Diff line change
`@@ -128,7 +128,7 @@ public final AggregatorFunctionSupplier supplier(List<Integer> inputChannels) {`
`128`	`128`	`if (type == DataType.IP) {`
`129`	`129`	`return new MaxIpAggregatorFunctionSupplier(inputChannels);`
`130`	`130`	`}`
`131`		`- if (type == DataType.VERSION \|\| type == DataType.KEYWORD \|\| type == DataType.TEXT) {`
	`131`	`+ if (type == DataType.VERSION \|\| DataType.isString(type)) {`
`132`	`132`	`return new MaxBytesRefAggregatorFunctionSupplier(inputChannels);`
`133`	`133`	`}`
`134`	`134`	`throw EsqlIllegalArgumentException.illegalDataType(type);`
Original file line number	Diff line number	Diff line change
`@@ -155,11 +155,7 @@ public ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {`
`155`	`155`	`if (dataType == DataType.LONG \|\| dataType == DataType.DATETIME) {`
`156`	`156`	`return new GreatestLongEvaluator.Factory(source(), factories);`
`157`	`157`	`}`
`158`		`- if (dataType == DataType.KEYWORD`
`159`		`- \|\| dataType == DataType.TEXT`
`160`		`- \|\| dataType == DataType.IP`
`161`		`- \|\| dataType == DataType.VERSION`
`162`		`- \|\| dataType == DataType.UNSUPPORTED) {`
	`158`	`+ if (DataType.isString(dataType) \|\| dataType == DataType.IP \|\| dataType == DataType.VERSION \|\| dataType == DataType.UNSUPPORTED) {`
`163`	`159`
`164`	`160`	`return new GreatestBytesRefEvaluator.Factory(source(), factories);`
`165`	`161`	`}`
Original file line number	Diff line number	Diff line change
`@@ -154,11 +154,7 @@ public ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {`
`154`	`154`	`if (dataType == DataType.LONG \|\| dataType == DataType.DATETIME) {`
`155`	`155`	`return new LeastLongEvaluator.Factory(source(), factories);`
`156`	`156`	`}`
`157`		`- if (dataType == DataType.KEYWORD`
`158`		`- \|\| dataType == DataType.TEXT`
`159`		`- \|\| dataType == DataType.IP`
`160`		`- \|\| dataType == DataType.VERSION`
`161`		`- \|\| dataType == DataType.UNSUPPORTED) {`
	`157`	`+ if (DataType.isString(dataType) \|\| dataType == DataType.IP \|\| dataType == DataType.VERSION \|\| dataType == DataType.UNSUPPORTED) {`
`162`	`158`
`163`	`159`	`return new LeastBytesRefEvaluator.Factory(source(), factories);`
`164`	`160`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1283,13 +1283,6 @@ public void allMemoryReleased() {`
`1283`	`1283`	`}`
`1284`	`1284`	`}`
`1285`	`1285`
`1286`		`- /**`
`1287`		`- * All string types (keyword, text, match_only_text, etc).`
`1288`		`- */`
`1289`		`- protected static DataType[] strings() {`
`1290`		`- return DataType.types().stream().filter(DataType::isString).toArray(DataType[]::new);`
`1291`		`- }`
`1292`		`-`
`1293`	`1286`	`/**`
`1294`	`1287`	`* Validate that we know the types for all the test cases already created`
`1295`	`1288`	`* @param suppliers - list of suppliers before adding in the illegal type combinations`
`@@ -1316,10 +1309,9 @@ private static boolean isAggregation() {`
`1316`	`1309`	`*/`
`1317`	`1310`	`private static boolean shouldHideSignature(List<DataType> argTypes, DataType returnType) {`
`1318`	`1311`	`for (DataType dt : DataType.UNDER_CONSTRUCTION.keySet()) {`
`1319`		`- if (returnType == dt) {`
	`1312`	`+ if (returnType == dt \|\| argTypes.contains(dt)) {`
`1320`	`1313`	`return true;`
`1321`	`1314`	`}`
`1322`		`- return argTypes.contains(dt);`
`1323`	`1315`	`}`
`1324`	`1316`	`return false;`
`1325`	`1317`	`}`