ESQL: Evaluator check function tests (#117715) (#118316)

Multiple scalar function tests were being ignored if they had any non-representable child (counters, durations, periods...). Here, I'm making that "can have an evaluator" a bit more explicit in every test, as many of the ignored tests cases were actually "working".
elastic · Dec 10, 2024 · 5c9cd6c · 5c9cd6c
1 parent 0e4de18
commit 5c9cd6c
Show file tree

Hide file tree

Showing 10 changed files with 42 additions and 41 deletions.
diff --git a/...java/org/elasticsearch/xpack/esql/expression/function/AbstractScalarFunctionTestCase.java b/...java/org/elasticsearch/xpack/esql/expression/function/AbstractScalarFunctionTestCase.java
@@ -104,7 +104,6 @@ public final void testEvaluate() {
             assertTypeResolutionFailure(expression);
             return;
         }
-        assumeTrue("Expected type must be representable to build an evaluator", DataType.isRepresentable(testCase.expectedType()));
         logger.info(
             "Test Values: " + testCase.getData().stream().map(TestCaseSupplier.TypedData::toString).collect(Collectors.joining(","))
         );
@@ -209,7 +208,6 @@ private void testEvaluateBlock(BlockFactory inputBlockFactory, DriverContext con
             return;
         }
         assumeTrue("Can't build evaluator", testCase.canBuildEvaluator());
-        assumeTrue("Expected type must be representable to build an evaluator", DataType.isRepresentable(testCase.expectedType()));
         int positions = between(1, 1024);
         List<TestCaseSupplier.TypedData> data = testCase.getData();
         Page onePositionPage = row(testCase.getDataValues());
@@ -275,7 +273,6 @@ public final void testEvaluateInManyThreads() throws ExecutionException, Interru
             return;
         }
         assumeTrue("Can't build evaluator", testCase.canBuildEvaluator());
-        assumeTrue("Expected type must be representable to build an evaluator", DataType.isRepresentable(testCase.expectedType()));
         int count = 10_000;
         int threads = 5;
         var evalSupplier = evaluator(expression);
@@ -338,14 +335,13 @@ public final void testFactoryToString() {
         assertThat(factory.toString(), testCase.evaluatorToString());
     }
 
-    public final void testFold() {
+    public void testFold() {
         Expression expression = buildLiteralExpression(testCase);
         if (testCase.getExpectedTypeError() != null) {
             assertTypeResolutionFailure(expression);
             return;
         }
         assertFalse("expected resolved", expression.typeResolved().unresolved());
-        assumeTrue("Can't build evaluator", testCase.canBuildEvaluator());
         Expression nullOptimized = new FoldNull().rule(expression);
         assertThat(nullOptimized.dataType(), equalTo(testCase.expectedType()));
         assertTrue(nullOptimized.foldable());

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
@@ -278,6 +278,9 @@ private static TestCaseSupplier testCaseSupplier(
             for (String warning : warnings.apply(lhsTyped, rhsTyped)) {
                 testCase = testCase.withWarning(warning);
             }
+            if (DataType.isRepresentable(expectedType) == false) {
+                testCase = testCase.withoutEvaluator();
+            }
             return testCase;
         });
     }
@@ -1438,7 +1441,7 @@ public static TestCase typeError(List<TypedData> data, String expectedTypeError)
                 foldingExceptionClass,
                 foldingExceptionMessage,
                 extra,
-                data.stream().allMatch(d -> d.forceLiteral || DataType.isRepresentable(d.type))
+                true
             );
         }
 

diff --git a/.../test/java/org/elasticsearch/xpack/esql/expression/function/grouping/CategorizeTests.java b/.../test/java/org/elasticsearch/xpack/esql/expression/function/grouping/CategorizeTests.java
@@ -58,4 +58,9 @@ public static Iterable<Object[]> parameters() {
     protected Expression build(Source source, List<Expression> args) {
         return new Categorize(source, args.get(0));
     }
+
+    @Override
+    public void testFold() {
+        // Cannot be folded
+    }
 }
diff --git a/...va/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToDatePeriodTests.java b/...va/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToDatePeriodTests.java
@@ -50,7 +50,7 @@ public static Iterable<Object[]> parameters() {
                 matchesPattern("LiteralsEvaluator.*"),
                 DATE_PERIOD,
                 equalTo(field)
-            );
+            ).withoutEvaluator();
         }));
 
         for (EsqlDataTypeConverter.INTERVALS interval : DATE_PERIODS) {
@@ -67,7 +67,7 @@ public static Iterable<Object[]> parameters() {
                         matchesPattern("LiteralsEvaluator.*"),
                         DATE_PERIOD,
                         equalTo(result)
-                    );
+                    ).withoutEvaluator();
                 }));
             }
         }

diff --git a/...t/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToDoubleTests.java b/...t/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToDoubleTests.java
@@ -126,7 +126,7 @@ public static Iterable<Object[]> parameters() {
         );
         TestCaseSupplier.unary(
             suppliers,
-            evaluatorName.apply("Integer"),
+            evaluatorName.apply("Int"),
             List.of(new TestCaseSupplier.TypedDataSupplier("counter", () -> randomInt(1000), DataType.COUNTER_INTEGER)),
             DataType.DOUBLE,
             l -> ((Integer) l).doubleValue(),

diff --git a/...est/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToLongTests.java b/...est/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToLongTests.java
@@ -230,7 +230,7 @@ public static Iterable<Object[]> parameters() {
         );
         TestCaseSupplier.unary(
             suppliers,
-            evaluatorName.apply("Integer"),
+            evaluatorName.apply("Int"),
             List.of(new TestCaseSupplier.TypedDataSupplier("counter", ESTestCase::randomInt, DataType.COUNTER_INTEGER)),
             DataType.LONG,
             l -> ((Integer) l).longValue(),

diff --git a/.../org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToTimeDurationTests.java b/.../org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToTimeDurationTests.java
@@ -50,7 +50,7 @@ public static Iterable<Object[]> parameters() {
                 matchesPattern("LiteralsEvaluator.*"),
                 TIME_DURATION,
                 equalTo(field)
-            );
+            ).withoutEvaluator();
         }));
 
         for (EsqlDataTypeConverter.INTERVALS interval : TIME_DURATIONS) {
@@ -66,7 +66,7 @@ public static Iterable<Object[]> parameters() {
                         matchesPattern("LiteralsEvaluator.*"),
                         TIME_DURATION,
                         equalTo(result)
-                    );
+                    ).withoutEvaluator();
                 }));
             }
         }

diff --git a/...est/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateTruncTests.java b/...est/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateTruncTests.java
@@ -136,10 +136,10 @@ private static TestCaseSupplier randomSecond() {
                 + pad(randomIntBetween(0, 59));
             return new TestCaseSupplier.TestCase(
                 List.of(
-                    new TestCaseSupplier.TypedData(Duration.ofSeconds(1), DataType.TIME_DURATION, "interval"),
+                    new TestCaseSupplier.TypedData(Duration.ofSeconds(1), DataType.TIME_DURATION, "interval").forceLiteral(),
                     new TestCaseSupplier.TypedData(toMillis(dateFragment + ".38Z"), DataType.DATETIME, "date")
                 ),
-                "DateTruncDatetimeEvaluator[date=Attribute[channel=1], interval=Attribute[channel=0]]",
+                Matchers.startsWith("DateTruncDatetimeEvaluator[fieldVal=Attribute[channel=0], rounding=Rounding["),
                 DataType.DATETIME,
                 equalTo(toMillis(dateFragment + ".00Z"))
             );

diff --git a/.../java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/NegTests.java b/.../java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/NegTests.java
@@ -10,22 +10,21 @@
 import com.carrotsearch.randomizedtesting.annotations.Name;
 import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
 
-import org.elasticsearch.compute.data.Block;
 import org.elasticsearch.xpack.esql.VerificationException;
 import org.elasticsearch.xpack.esql.core.expression.Expression;
 import org.elasticsearch.xpack.esql.core.expression.Literal;
 import org.elasticsearch.xpack.esql.core.tree.Source;
 import org.elasticsearch.xpack.esql.core.type.DataType;
 import org.elasticsearch.xpack.esql.expression.function.AbstractScalarFunctionTestCase;
 import org.elasticsearch.xpack.esql.expression.function.TestCaseSupplier;
+import org.hamcrest.Matchers;
 
 import java.time.Duration;
 import java.time.Period;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.function.Supplier;
 
-import static org.elasticsearch.compute.data.BlockUtils.toJavaObject;
 import static org.elasticsearch.xpack.esql.EsqlTestUtils.randomLiteral;
 import static org.hamcrest.Matchers.equalTo;
 
@@ -97,19 +96,19 @@ public static Iterable<Object[]> parameters() {
         suppliers.addAll(List.of(new TestCaseSupplier("Duration", List.of(DataType.TIME_DURATION), () -> {
             Duration arg = (Duration) randomLiteral(DataType.TIME_DURATION).value();
             return new TestCaseSupplier.TestCase(
-                List.of(new TestCaseSupplier.TypedData(arg, DataType.TIME_DURATION, "arg")),
-                "No evaluator since this expression is only folded",
+                List.of(new TestCaseSupplier.TypedData(arg, DataType.TIME_DURATION, "arg").forceLiteral()),
+                Matchers.startsWith("LiteralsEvaluator[lit="),
                 DataType.TIME_DURATION,
                 equalTo(arg.negated())
-            );
+            ).withoutEvaluator();
         }), new TestCaseSupplier("Period", List.of(DataType.DATE_PERIOD), () -> {
             Period arg = (Period) randomLiteral(DataType.DATE_PERIOD).value();
             return new TestCaseSupplier.TestCase(
-                List.of(new TestCaseSupplier.TypedData(arg, DataType.DATE_PERIOD, "arg")),
-                "No evaluator since this expression is only folded",
+                List.of(new TestCaseSupplier.TypedData(arg, DataType.DATE_PERIOD, "arg").forceLiteral()),
+                Matchers.startsWith("LiteralsEvaluator[lit="),
                 DataType.DATE_PERIOD,
                 equalTo(arg.negated())
-            );
+            ).withoutEvaluator();
         })));
         return parameterSuppliersFromTypedDataWithDefaultChecks(false, suppliers, (v, p) -> "numeric, date_period or time_duration");
     }
@@ -126,25 +125,25 @@ public void testEdgeCases() {
         if (testCaseType == DataType.DATE_PERIOD) {
             Period maxPeriod = Period.of(Integer.MAX_VALUE, Integer.MAX_VALUE, Integer.MAX_VALUE);
             Period negatedMaxPeriod = Period.of(-Integer.MAX_VALUE, -Integer.MAX_VALUE, -Integer.MAX_VALUE);
-            assertEquals(negatedMaxPeriod, process(maxPeriod));
+            assertEquals(negatedMaxPeriod, foldTemporalAmount(maxPeriod));
 
             Period minPeriod = Period.of(Integer.MIN_VALUE, Integer.MIN_VALUE, Integer.MIN_VALUE);
             VerificationException e = expectThrows(
                 VerificationException.class,
                 "Expected exception when negating minimal date period.",
-                () -> process(minPeriod)
+                () -> foldTemporalAmount(minPeriod)
             );
             assertEquals(e.getMessage(), "arithmetic exception in expression []: [integer overflow]");
         } else if (testCaseType == DataType.TIME_DURATION) {
             Duration maxDuration = Duration.ofSeconds(Long.MAX_VALUE, 0);
             Duration negatedMaxDuration = Duration.ofSeconds(-Long.MAX_VALUE, 0);
-            assertEquals(negatedMaxDuration, process(maxDuration));
+            assertEquals(negatedMaxDuration, foldTemporalAmount(maxDuration));
 
             Duration minDuration = Duration.ofSeconds(Long.MIN_VALUE, 0);
             VerificationException e = expectThrows(
                 VerificationException.class,
                 "Expected exception when negating minimal time duration.",
-                () -> process(minDuration)
+                () -> foldTemporalAmount(minDuration)
             );
             assertEquals(
                 e.getMessage(),
@@ -153,16 +152,9 @@ public void testEdgeCases() {
         }
     }
 
-    private Object process(Object val) {
-        if (testCase.canBuildEvaluator()) {
-            Neg neg = new Neg(Source.EMPTY, field("val", typeOf(val)));
-            try (Block block = evaluator(neg).get(driverContext()).eval(row(List.of(val)))) {
-                return toJavaObject(block, 0);
-            }
-        } else { // just fold if type is not representable
-            Neg neg = new Neg(Source.EMPTY, new Literal(Source.EMPTY, val, typeOf(val)));
-            return neg.fold();
-        }
+    private Object foldTemporalAmount(Object val) {
+        Neg neg = new Neg(Source.EMPTY, new Literal(Source.EMPTY, val, typeOf(val)));
+        return neg.fold();
     }
 
     private static DataType typeOf(Object val) {

diff --git a/.../java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/SubTests.java b/.../java/org/elasticsearch/xpack/esql/expression/predicate/operator/arithmetic/SubTests.java
@@ -169,12 +169,12 @@ public static Iterable<Object[]> parameters() {
             return new TestCaseSupplier.TestCase(
                 List.of(
                     new TestCaseSupplier.TypedData(lhs, DataType.DATE_PERIOD, "lhs"),
-                    new TestCaseSupplier.TypedData(rhs, DataType.DATE_PERIOD, "rhs")
+                    new TestCaseSupplier.TypedData(rhs, DataType.DATE_PERIOD, "rhs").forceLiteral()
                 ),
                 "Only folding possible, so there's no evaluator",
                 DataType.DATE_PERIOD,
                 equalTo(lhs.minus(rhs))
-            );
+            ).withoutEvaluator();
         }));
         suppliers.add(new TestCaseSupplier("Datetime - Duration", List.of(DataType.DATETIME, DataType.TIME_DURATION), () -> {
             long lhs = (Long) randomLiteral(DataType.DATETIME).value();
@@ -196,12 +196,12 @@ public static Iterable<Object[]> parameters() {
             return new TestCaseSupplier.TestCase(
                 List.of(
                     new TestCaseSupplier.TypedData(lhs, DataType.TIME_DURATION, "lhs"),
-                    new TestCaseSupplier.TypedData(rhs, DataType.TIME_DURATION, "rhs")
+                    new TestCaseSupplier.TypedData(rhs, DataType.TIME_DURATION, "rhs").forceLiteral()
                 ),
                 "Only folding possible, so there's no evaluator",
                 DataType.TIME_DURATION,
                 equalTo(lhs.minus(rhs))
-            );
+            ).withoutEvaluator();
         }));
 
         // exact math arithmetic exceptions
@@ -250,7 +250,12 @@ public static Iterable<Object[]> parameters() {
                 return original.getData().get(nullPosition == 0 ? 1 : 0).type();
             }
             return original.expectedType();
-        }, (nullPosition, nullData, original) -> nullData.isForceLiteral() ? equalTo("LiteralsEvaluator[lit=null]") : original);
+        }, (nullPosition, nullData, original) -> {
+            if (DataType.isTemporalAmount(nullData.type())) {
+                return equalTo("LiteralsEvaluator[lit=null]");
+            }
+            return original;
+        });
 
         suppliers.add(new TestCaseSupplier("MV", List.of(DataType.INTEGER, DataType.INTEGER), () -> {
             // Ensure we don't have an overflow