Simplify the code to check if a return type is Text or not

craigtaverner · Oct 22, 2024 · 9b31a8f · 9b31a8f
1 parent bb1edf9
commit 9b31a8f
Show file tree

Hide file tree

Showing 13 changed files with 16 additions and 34 deletions.
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
@@ -584,6 +584,10 @@ static Builder builder() {
         return new Builder();
     }
 
+    public DataType noText() {
+        return this == DataType.TEXT ? DataType.KEYWORD : this;
+    }
+
     /**
      * Named parameters with default values. It's just easier to do this with
      * a builder in java....

diff --git a/...ain/java/org/elasticsearch/xpack/esql/expression/function/scalar/UnaryScalarFunction.java b/...ain/java/org/elasticsearch/xpack/esql/expression/function/scalar/UnaryScalarFunction.java
@@ -164,6 +164,6 @@ public final Expression field() {
 
     @Override
     public DataType dataType() {
-        return field.dataType() == DataType.TEXT ? DataType.KEYWORD : field.dataType();
+        return field.dataType().noText();
     }
 }
diff --git a/...ql/src/test/java/org/elasticsearch/xpack/esql/expression/function/aggregate/MaxTests.java b/...ql/src/test/java/org/elasticsearch/xpack/esql/expression/function/aggregate/MaxTests.java
@@ -172,7 +172,7 @@ private static TestCaseSupplier makeSupplier(TestCaseSupplier.TypedDataSupplier
             return new TestCaseSupplier.TestCase(
                 List.of(fieldTypedData),
                 "Max[field=Attribute[channel=0]]",
-                fieldSupplier.type() == DataType.TEXT ? DataType.KEYWORD : fieldSupplier.type(),
+                fieldSupplier.type().noText(),
                 equalTo(expected)
             );
         });

diff --git a/...ql/src/test/java/org/elasticsearch/xpack/esql/expression/function/aggregate/MinTests.java b/...ql/src/test/java/org/elasticsearch/xpack/esql/expression/function/aggregate/MinTests.java
@@ -172,7 +172,7 @@ private static TestCaseSupplier makeSupplier(TestCaseSupplier.TypedDataSupplier
             return new TestCaseSupplier.TestCase(
                 List.of(fieldTypedData),
                 "Min[field=Attribute[channel=0]]",
-                fieldSupplier.type() == DataType.TEXT ? DataType.KEYWORD : fieldSupplier.type(),
+                fieldSupplier.type().noText(),
                 equalTo(expected)
             );
         });

diff --git a/...ql/src/test/java/org/elasticsearch/xpack/esql/expression/function/aggregate/TopTests.java b/...ql/src/test/java/org/elasticsearch/xpack/esql/expression/function/aggregate/TopTests.java
@@ -311,7 +311,7 @@ private static TestCaseSupplier makeSupplier(
                     new TestCaseSupplier.TypedData(new BytesRef(order), DataType.KEYWORD, order + " order").forceLiteral()
                 ),
                 "Top[field=Attribute[channel=0], limit=Attribute[channel=1], order=Attribute[channel=2]]",
-                fieldSupplier.type() == DataType.TEXT ? DataType.KEYWORD : fieldSupplier.type(),
+                fieldSupplier.type().noText(),
                 equalTo(expected.size() == 1 ? expected.get(0) : expected)
             );
         });

diff --git a/...src/test/java/org/elasticsearch/xpack/esql/expression/function/aggregate/ValuesTests.java b/...src/test/java/org/elasticsearch/xpack/esql/expression/function/aggregate/ValuesTests.java
@@ -78,7 +78,7 @@ private static TestCaseSupplier makeSupplier(TestCaseSupplier.TypedDataSupplier
             return new TestCaseSupplier.TestCase(
                 List.of(fieldTypedData),
                 "Values[field=Attribute[channel=0]]",
-                fieldSupplier.type() == DataType.TEXT ? DataType.KEYWORD : fieldSupplier.type(),
+                fieldSupplier.type().noText(),
                 expected.isEmpty() ? nullValue() : valuesInAnyOrder(expected)
             );
         });

diff --git a/.../xpack/esql/expression/function/scalar/multivalue/AbstractMultivalueFunctionTestCase.java b/.../xpack/esql/expression/function/scalar/multivalue/AbstractMultivalueFunctionTestCase.java
@@ -110,7 +110,7 @@ protected static void bytesRefs(
         String evaluatorName,
         BiFunction<Integer, Stream<BytesRef>, Matcher<Object>> matcher
     ) {
-        bytesRefs(cases, name, evaluatorName, t -> t == DataType.TEXT ? DataType.KEYWORD : t, matcher);
+        bytesRefs(cases, name, evaluatorName, DataType::noText, matcher);
     }
 
     /**

diff --git a/...java/org/elasticsearch/xpack/esql/expression/function/scalar/multivalue/MvFirstTests.java b/...java/org/elasticsearch/xpack/esql/expression/function/scalar/multivalue/MvFirstTests.java
@@ -30,13 +30,7 @@ public MvFirstTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> testCa
     public static Iterable<Object[]> parameters() {
         List<TestCaseSupplier> cases = new ArrayList<>();
         booleans(cases, "mv_first", "MvFirst", DataType.BOOLEAN, (size, values) -> equalTo(values.findFirst().get()));
-        bytesRefs(
-            cases,
-            "mv_first",
-            "MvFirst",
-            x -> x == DataType.TEXT ? DataType.KEYWORD : x,
-            (size, values) -> equalTo(values.findFirst().get())
-        );
+        bytesRefs(cases, "mv_first", "MvFirst", DataType::noText, (size, values) -> equalTo(values.findFirst().get()));
         doubles(cases, "mv_first", "MvFirst", DataType.DOUBLE, (size, values) -> equalTo(values.findFirst().getAsDouble()));
         ints(cases, "mv_first", "MvFirst", DataType.INTEGER, (size, values) -> equalTo(values.findFirst().getAsInt()));
         longs(cases, "mv_first", "MvFirst", DataType.LONG, (size, values) -> equalTo(values.findFirst().getAsLong()));

diff --git a/.../java/org/elasticsearch/xpack/esql/expression/function/scalar/multivalue/MvLastTests.java b/.../java/org/elasticsearch/xpack/esql/expression/function/scalar/multivalue/MvLastTests.java
@@ -30,13 +30,7 @@ public MvLastTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> testCas
     public static Iterable<Object[]> parameters() {
         List<TestCaseSupplier> cases = new ArrayList<>();
         booleans(cases, "mv_last", "MvLast", DataType.BOOLEAN, (size, values) -> equalTo(values.reduce((f, s) -> s).get()));
-        bytesRefs(
-            cases,
-            "mv_last",
-            "MvLast",
-            x -> x == DataType.TEXT ? DataType.KEYWORD : x,
-            (size, values) -> equalTo(values.reduce((f, s) -> s).get())
-        );
+        bytesRefs(cases, "mv_last", "MvLast", DataType::noText, (size, values) -> equalTo(values.reduce((f, s) -> s).get()));
         doubles(cases, "mv_last", "MvLast", DataType.DOUBLE, (size, values) -> equalTo(values.reduce((f, s) -> s).getAsDouble()));
         ints(cases, "mv_last", "MvLast", DataType.INTEGER, (size, values) -> equalTo(values.reduce((f, s) -> s).getAsInt()));
         longs(cases, "mv_last", "MvLast", DataType.LONG, (size, values) -> equalTo(values.reduce((f, s) -> s).getAsLong()));

diff --git a/...ava/org/elasticsearch/xpack/esql/expression/function/scalar/string/AbstractTrimTests.java b/...ava/org/elasticsearch/xpack/esql/expression/function/scalar/string/AbstractTrimTests.java
@@ -74,7 +74,7 @@ private static TestCaseSupplier.TestCase testCase(String name, DataType type, St
         return new TestCaseSupplier.TestCase(
             List.of(new TestCaseSupplier.TypedData(new BytesRef(data), type, "str")),
             name + "[val=Attribute[channel=0]]",
-            type == DataType.TEXT ? DataType.KEYWORD : type,
+            type.noText(),
             equalTo(new BytesRef(expected))
         );
     }

diff --git a/...est/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/ReverseTests.java b/...est/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/ReverseTests.java
@@ -57,7 +57,7 @@ private static TestCaseSupplier makeSupplier(TestCaseSupplier.TypedDataSupplier
             return new TestCaseSupplier.TestCase(
                 List.of(fieldTypedData),
                 expectedToString,
-                fieldSupplier.type() == DataType.TEXT ? DataType.KEYWORD : fieldSupplier.type(),
+                fieldSupplier.type().noText(),
                 equalTo(new BytesRef(expectedValue))
             );
         });

diff --git a/...est/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/ToLowerTests.java b/...est/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/ToLowerTests.java
@@ -87,12 +87,7 @@ private static TestCaseSupplier supplier(String name, DataType type, Supplier<St
             values.add(new TestCaseSupplier.TypedData(new BytesRef(value), type, "0"));
 
             String expectedValue = value.toLowerCase(EsqlTestUtils.TEST_CFG.locale());
-            return new TestCaseSupplier.TestCase(
-                values,
-                expectedToString,
-                type == DataType.TEXT ? DataType.KEYWORD : type,
-                equalTo(new BytesRef(expectedValue))
-            );
+            return new TestCaseSupplier.TestCase(values, expectedToString, type.noText(), equalTo(new BytesRef(expectedValue)));
         });
     }
 }
diff --git a/...est/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/ToUpperTests.java b/...est/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/ToUpperTests.java
@@ -87,12 +87,7 @@ private static TestCaseSupplier supplier(String name, DataType type, Supplier<St
             values.add(new TestCaseSupplier.TypedData(new BytesRef(value), type, "0"));
 
             String expectedValue = value.toUpperCase(EsqlTestUtils.TEST_CFG.locale());
-            return new TestCaseSupplier.TestCase(
-                values,
-                expectedToString,
-                type == DataType.TEXT ? DataType.KEYWORD : type,
-                equalTo(new BytesRef(expectedValue))
-            );
+            return new TestCaseSupplier.TestCase(values, expectedToString, type.noText(), equalTo(new BytesRef(expectedValue)));
         });
     }
 }