Просмотр исходного кода

Esql - rework date diff tests (#120328) (#120408)

In preparation for adding nanosecond date support to date diff, this PR expands the test coverage and better integrates with the test generation framework. Using random data for date diff is extremely difficult, due to the vast range of units and the constraints of the integer return type. For example, nanosecond resolution differences of more than a couple of seconds will overflow the return value. So, instead of using a fully randomized dataset, I took the existing data set we were running against just the processing function, and formalized that to run against the whole test suite. It should be easy from here to add in cases for the same values as nanoseconds.

I was able to integrate the overflow error case into the normal test framework, but the incorrect unit error doesn't play nicely with the framework (I believe this is because it fails in toEvaluator rather than in process, but I didn't dig in much). I've moved it to another test file so we don't run it for every case in the parameterized test file.

---------

Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co>
Mark Tozzi 8 месяцев назад
Родитель
Сommit
4b96c73146

+ 40 - 0
x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateDiffFunctionTests.java

@@ -0,0 +1,40 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+package org.elasticsearch.xpack.esql.expression.function.scalar.date;
+
+import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.test.ESTestCase;
+
+import static org.hamcrest.Matchers.containsString;
+
+/**
+ * Tests for {@link DateDiff} that should not run through the normal testing framework
+ */
+public class DateDiffFunctionTests extends ESTestCase {
+
+    public void testDateDiffFunctionErrorUnitNotValid() {
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> DateDiff.process(new BytesRef("sseconds"), 0, 0));
+        assertThat(
+            e.getMessage(),
+            containsString(
+                "Received value [sseconds] is not valid date part to add; "
+                    + "did you mean [seconds, second, nanoseconds, milliseconds, microseconds, nanosecond]?"
+            )
+        );
+
+        e = expectThrows(IllegalArgumentException.class, () -> DateDiff.process(new BytesRef("not-valid-unit"), 0, 0));
+        assertThat(
+            e.getMessage(),
+            containsString(
+                "A value of [YEAR, QUARTER, MONTH, DAYOFYEAR, DAY, WEEK, WEEKDAY, HOUR, MINUTE, SECOND, MILLISECOND, MICROSECOND, "
+                    + "NANOSECOND] or their aliases is required; received [not-valid-unit]"
+            )
+        );
+    }
+
+}

+ 115 - 122
x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/date/DateDiffTests.java

@@ -11,19 +11,17 @@ import com.carrotsearch.randomizedtesting.annotations.Name;
 import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
 
 import org.apache.lucene.util.BytesRef;
-import org.elasticsearch.xpack.esql.core.InvalidArgumentException;
 import org.elasticsearch.xpack.esql.core.expression.Expression;
 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 java.time.ZonedDateTime;
+import java.time.Instant;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.function.Supplier;
 
-import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 
 public class DateDiffTests extends AbstractScalarFunctionTestCase {
@@ -33,149 +31,144 @@ public class DateDiffTests extends AbstractScalarFunctionTestCase {
 
     @ParametersFactory
     public static Iterable<Object[]> parameters() {
-        ZonedDateTime zdtStart = ZonedDateTime.parse("2023-12-04T10:15:30Z");
-        ZonedDateTime zdtEnd = ZonedDateTime.parse("2023-12-05T10:45:00Z");
 
         List<TestCaseSupplier> suppliers = new ArrayList<>();
-        suppliers.add(
+        suppliers.addAll(makeSuppliers(Instant.parse("2023-12-04T10:15:30Z"), Instant.parse("2023-12-05T10:45:00Z"), "seconds", 88170));
+        suppliers.addAll(makeSuppliers(Instant.parse("2023-12-12T00:01:01Z"), Instant.parse("2024-12-12T00:01:01Z"), "year", 1));
+        suppliers.addAll(makeSuppliers(Instant.parse("2023-12-12T00:01:01.001Z"), Instant.parse("2024-12-12T00:01:01Z"), "year", 0));
+
+        suppliers.addAll(
+            makeSuppliers(Instant.parse("2023-12-04T10:15:00Z"), Instant.parse("2023-12-04T10:15:01Z"), "nanoseconds", 1000000000)
+        );
+        suppliers.addAll(makeSuppliers(Instant.parse("2023-12-04T10:15:00Z"), Instant.parse("2023-12-04T10:15:01Z"), "ns", 1000000000));
+        suppliers.addAll(
+            makeSuppliers(Instant.parse("2023-12-04T10:15:00Z"), Instant.parse("2023-12-04T10:15:01Z"), "microseconds", 1000000)
+        );
+        suppliers.addAll(makeSuppliers(Instant.parse("2023-12-04T10:15:00Z"), Instant.parse("2023-12-04T10:15:01Z"), "mcs", 1000000));
+        suppliers.addAll(makeSuppliers(Instant.parse("2023-12-04T10:15:00Z"), Instant.parse("2023-12-04T10:15:01Z"), "milliseconds", 1000));
+        suppliers.addAll(makeSuppliers(Instant.parse("2023-12-04T10:15:00Z"), Instant.parse("2023-12-04T10:15:01Z"), "ms", 1000));
+        suppliers.addAll(makeSuppliers(Instant.parse("2023-12-04T10:15:00Z"), Instant.parse("2023-12-04T10:15:01Z"), "seconds", 1));
+        suppliers.addAll(makeSuppliers(Instant.parse("2023-12-04T10:15:00Z"), Instant.parse("2023-12-04T10:15:01Z"), "ss", 1));
+        suppliers.addAll(makeSuppliers(Instant.parse("2023-12-04T10:15:00Z"), Instant.parse("2023-12-04T10:15:01Z"), "s", 1));
+
+        Instant zdtStart = Instant.parse("2023-12-04T10:15:00Z");
+        Instant zdtEnd = Instant.parse("2024-12-04T10:15:01Z");
+
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "minutes", 527040));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "mi", 527040));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "n", 527040));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "hours", 8784));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "hh", 8784));
+
+        // 2024 is a leap year, so the dates are 366 days apart
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "weekdays", 366));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "dw", 366));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "days", 366));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "dd", 366));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "d", 366));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "dy", 366));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "y", 366));
+
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "weeks", 52));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "wk", 52));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "ww", 52));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "months", 12));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "mm", 12));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "m", 12));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "quarters", 4));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "qq", 4));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "q", 4));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "years", 1));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "yyyy", 1));
+        suppliers.addAll(makeSuppliers(zdtStart, zdtEnd, "yy", 1));
+
+        // Error cases
+        Instant zdtStart2 = Instant.parse("2023-12-04T10:15:00Z");
+        Instant zdtEnd2 = Instant.parse("2023-12-04T10:20:00Z");
+        suppliers.addAll(
+            makeSuppliers(
+                zdtStart2,
+                zdtEnd2,
+                "nanoseconds",
+                "Line -1:-1: org.elasticsearch.xpack.esql.core.InvalidArgumentException: [300000000000] out of [integer] range"
+            )
+        );
+
+        return parameterSuppliersFromTypedDataWithDefaultChecksNoErrors(true, suppliers);
+    }
+
+    private static List<TestCaseSupplier> makeSuppliers(Instant startTimestamp, Instant endTimestamp, String unit, int expected) {
+        // Units as Keyword case
+        return List.of(
             new TestCaseSupplier(
-                "Date Diff In Seconds - OK",
+                "DateDiff(" + unit + "<KEYWORD>, " + startTimestamp + ", " + endTimestamp + ") == " + expected,
                 List.of(DataType.KEYWORD, DataType.DATETIME, DataType.DATETIME),
                 () -> new TestCaseSupplier.TestCase(
                     List.of(
-                        new TestCaseSupplier.TypedData(new BytesRef("seconds"), DataType.KEYWORD, "unit"),
-                        new TestCaseSupplier.TypedData(zdtStart.toInstant().toEpochMilli(), DataType.DATETIME, "startTimestamp"),
-                        new TestCaseSupplier.TypedData(zdtEnd.toInstant().toEpochMilli(), DataType.DATETIME, "endTimestamp")
+                        new TestCaseSupplier.TypedData(new BytesRef(unit), DataType.KEYWORD, "unit"),
+                        new TestCaseSupplier.TypedData(startTimestamp.toEpochMilli(), DataType.DATETIME, "startTimestamp"),
+                        new TestCaseSupplier.TypedData(endTimestamp.toEpochMilli(), DataType.DATETIME, "endTimestamp")
                     ),
                     "DateDiffEvaluator[unit=Attribute[channel=0], startTimestamp=Attribute[channel=1], "
                         + "endTimestamp=Attribute[channel=2]]",
                     DataType.INTEGER,
-                    equalTo(88170)
+                    equalTo(expected)
                 )
-            )
-        );
-        suppliers.add(
+            ),
+            // Units as text case
             new TestCaseSupplier(
-                "Date Diff In Seconds with text- OK",
+                "DateDiff(" + unit + "<TEXT>, " + startTimestamp + ", " + endTimestamp + ") == " + expected,
                 List.of(DataType.TEXT, DataType.DATETIME, DataType.DATETIME),
                 () -> new TestCaseSupplier.TestCase(
                     List.of(
-                        new TestCaseSupplier.TypedData(new BytesRef("seconds"), DataType.TEXT, "unit"),
-                        new TestCaseSupplier.TypedData(zdtStart.toInstant().toEpochMilli(), DataType.DATETIME, "startTimestamp"),
-                        new TestCaseSupplier.TypedData(zdtEnd.toInstant().toEpochMilli(), DataType.DATETIME, "endTimestamp")
+                        new TestCaseSupplier.TypedData(new BytesRef(unit), DataType.TEXT, "unit"),
+                        new TestCaseSupplier.TypedData(startTimestamp.toEpochMilli(), DataType.DATETIME, "startTimestamp"),
+                        new TestCaseSupplier.TypedData(endTimestamp.toEpochMilli(), DataType.DATETIME, "endTimestamp")
                     ),
                     "DateDiffEvaluator[unit=Attribute[channel=0], startTimestamp=Attribute[channel=1], "
                         + "endTimestamp=Attribute[channel=2]]",
                     DataType.INTEGER,
-                    equalTo(88170)
+                    equalTo(expected)
                 )
             )
         );
-        suppliers.add(new TestCaseSupplier("Date Diff In Year - 1", List.of(DataType.KEYWORD, DataType.DATETIME, DataType.DATETIME), () -> {
-            ZonedDateTime zdtStart2 = ZonedDateTime.parse("2023-12-12T00:01:01Z");
-            ZonedDateTime zdtEnd2 = ZonedDateTime.parse("2024-12-12T00:01:01Z");
-            return new TestCaseSupplier.TestCase(
-                List.of(
-                    new TestCaseSupplier.TypedData(new BytesRef("year"), DataType.KEYWORD, "unit"),
-                    new TestCaseSupplier.TypedData(zdtStart2.toInstant().toEpochMilli(), DataType.DATETIME, "startTimestamp"),
-                    new TestCaseSupplier.TypedData(zdtEnd2.toInstant().toEpochMilli(), DataType.DATETIME, "endTimestamp")
-                ),
-                "DateDiffEvaluator[unit=Attribute[channel=0], startTimestamp=Attribute[channel=1], " + "endTimestamp=Attribute[channel=2]]",
-                DataType.INTEGER,
-                equalTo(1)
-            );
-        }));
-        suppliers.add(new TestCaseSupplier("Date Diff In Year - 0", List.of(DataType.KEYWORD, DataType.DATETIME, DataType.DATETIME), () -> {
-            ZonedDateTime zdtStart2 = ZonedDateTime.parse("2023-12-12T00:01:01.001Z");
-            ZonedDateTime zdtEnd2 = ZonedDateTime.parse("2024-12-12T00:01:01Z");
-            return new TestCaseSupplier.TestCase(
-                List.of(
-                    new TestCaseSupplier.TypedData(new BytesRef("year"), DataType.KEYWORD, "unit"),
-                    new TestCaseSupplier.TypedData(zdtStart2.toInstant().toEpochMilli(), DataType.DATETIME, "startTimestamp"),
-                    new TestCaseSupplier.TypedData(zdtEnd2.toInstant().toEpochMilli(), DataType.DATETIME, "endTimestamp")
-                ),
-                "DateDiffEvaluator[unit=Attribute[channel=0], startTimestamp=Attribute[channel=1], " + "endTimestamp=Attribute[channel=2]]",
-                DataType.INTEGER,
-                equalTo(0)
-            );
-        }));
-        return parameterSuppliersFromTypedDataWithDefaultChecksNoErrors(true, suppliers);
     }
 
-    public void testDateDiffFunction() {
-        ZonedDateTime zdtStart = ZonedDateTime.parse("2023-12-04T10:15:00Z");
-        ZonedDateTime zdtEnd = ZonedDateTime.parse("2023-12-04T10:15:01Z");
-        long startTimestamp = zdtStart.toInstant().toEpochMilli();
-        long endTimestamp = zdtEnd.toInstant().toEpochMilli();
-
-        assertEquals(1000000000, DateDiff.process(new BytesRef("nanoseconds"), startTimestamp, endTimestamp));
-        assertEquals(1000000000, DateDiff.process(new BytesRef("ns"), startTimestamp, endTimestamp));
-        assertEquals(1000000, DateDiff.process(new BytesRef("microseconds"), startTimestamp, endTimestamp));
-        assertEquals(1000000, DateDiff.process(new BytesRef("mcs"), startTimestamp, endTimestamp));
-        assertEquals(1000, DateDiff.process(new BytesRef("milliseconds"), startTimestamp, endTimestamp));
-        assertEquals(1000, DateDiff.process(new BytesRef("ms"), startTimestamp, endTimestamp));
-        assertEquals(1, DateDiff.process(new BytesRef("seconds"), startTimestamp, endTimestamp));
-        assertEquals(1, DateDiff.process(new BytesRef("ss"), startTimestamp, endTimestamp));
-        assertEquals(1, DateDiff.process(new BytesRef("s"), startTimestamp, endTimestamp));
-
-        zdtEnd = zdtEnd.plusYears(1);
-        endTimestamp = zdtEnd.toInstant().toEpochMilli();
-
-        assertEquals(527040, DateDiff.process(new BytesRef("minutes"), startTimestamp, endTimestamp));
-        assertEquals(527040, DateDiff.process(new BytesRef("mi"), startTimestamp, endTimestamp));
-        assertEquals(527040, DateDiff.process(new BytesRef("n"), startTimestamp, endTimestamp));
-        assertEquals(8784, DateDiff.process(new BytesRef("hours"), startTimestamp, endTimestamp));
-        assertEquals(8784, DateDiff.process(new BytesRef("hh"), startTimestamp, endTimestamp));
-        assertEquals(366, DateDiff.process(new BytesRef("weekdays"), startTimestamp, endTimestamp));
-        assertEquals(366, DateDiff.process(new BytesRef("dw"), startTimestamp, endTimestamp));
-        assertEquals(52, DateDiff.process(new BytesRef("weeks"), startTimestamp, endTimestamp));
-        assertEquals(52, DateDiff.process(new BytesRef("wk"), startTimestamp, endTimestamp));
-        assertEquals(52, DateDiff.process(new BytesRef("ww"), startTimestamp, endTimestamp));
-        assertEquals(366, DateDiff.process(new BytesRef("days"), startTimestamp, endTimestamp));
-        assertEquals(366, DateDiff.process(new BytesRef("dd"), startTimestamp, endTimestamp));
-        assertEquals(366, DateDiff.process(new BytesRef("d"), startTimestamp, endTimestamp));
-        assertEquals(366, DateDiff.process(new BytesRef("dy"), startTimestamp, endTimestamp));
-        assertEquals(366, DateDiff.process(new BytesRef("y"), startTimestamp, endTimestamp));
-        assertEquals(12, DateDiff.process(new BytesRef("months"), startTimestamp, endTimestamp));
-        assertEquals(12, DateDiff.process(new BytesRef("mm"), startTimestamp, endTimestamp));
-        assertEquals(12, DateDiff.process(new BytesRef("m"), startTimestamp, endTimestamp));
-        assertEquals(4, DateDiff.process(new BytesRef("quarters"), startTimestamp, endTimestamp));
-        assertEquals(4, DateDiff.process(new BytesRef("qq"), startTimestamp, endTimestamp));
-        assertEquals(4, DateDiff.process(new BytesRef("q"), startTimestamp, endTimestamp));
-        assertEquals(1, DateDiff.process(new BytesRef("years"), startTimestamp, endTimestamp));
-        assertEquals(1, DateDiff.process(new BytesRef("yyyy"), startTimestamp, endTimestamp));
-        assertEquals(1, DateDiff.process(new BytesRef("yy"), startTimestamp, endTimestamp));
-    }
-
-    public void testDateDiffFunctionErrorTooLarge() {
-        ZonedDateTime zdtStart = ZonedDateTime.parse("2023-12-04T10:15:00Z");
-        ZonedDateTime zdtEnd = ZonedDateTime.parse("2023-12-04T10:20:00Z");
-        long startTimestamp = zdtStart.toInstant().toEpochMilli();
-        long endTimestamp = zdtEnd.toInstant().toEpochMilli();
-
-        InvalidArgumentException e = expectThrows(
-            InvalidArgumentException.class,
-            () -> DateDiff.process(new BytesRef("nanoseconds"), startTimestamp, endTimestamp)
-        );
-        assertThat(e.getMessage(), containsString("[300000000000] out of [integer] range"));
-    }
-
-    public void testDateDiffFunctionErrorUnitNotValid() {
-        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> DateDiff.process(new BytesRef("sseconds"), 0, 0));
-        assertThat(
-            e.getMessage(),
-            containsString(
-                "Received value [sseconds] is not valid date part to add; "
-                    + "did you mean [seconds, second, nanoseconds, milliseconds, microseconds, nanosecond]?"
-            )
-        );
-
-        e = expectThrows(IllegalArgumentException.class, () -> DateDiff.process(new BytesRef("not-valid-unit"), 0, 0));
-        assertThat(
-            e.getMessage(),
-            containsString(
-                "A value of [YEAR, QUARTER, MONTH, DAYOFYEAR, DAY, WEEK, WEEKDAY, HOUR, MINUTE, SECOND, MILLISECOND, MICROSECOND, "
-                    + "NANOSECOND] or their aliases is required; received [not-valid-unit]"
+    private static List<TestCaseSupplier> makeSuppliers(Instant startTimestamp, Instant endTimestamp, String unit, String warning) {
+        // Units as Keyword case
+        return List.of(
+            new TestCaseSupplier(
+                "DateDiff(" + unit + "<KEYWORD>, " + startTimestamp + ", " + endTimestamp + ") -> warning ",
+                List.of(DataType.KEYWORD, DataType.DATETIME, DataType.DATETIME),
+                () -> new TestCaseSupplier.TestCase(
+                    List.of(
+                        new TestCaseSupplier.TypedData(new BytesRef(unit), DataType.KEYWORD, "unit"),
+                        new TestCaseSupplier.TypedData(startTimestamp.toEpochMilli(), DataType.DATETIME, "startTimestamp"),
+                        new TestCaseSupplier.TypedData(endTimestamp.toEpochMilli(), DataType.DATETIME, "endTimestamp")
+                    ),
+                    "DateDiffEvaluator[unit=Attribute[channel=0], startTimestamp=Attribute[channel=1], "
+                        + "endTimestamp=Attribute[channel=2]]",
+                    DataType.INTEGER,
+                    equalTo(null)
+                ).withWarning("Line -1:-1: evaluation of [] failed, treating result as null. Only first 20 failures recorded.")
+                    .withWarning(warning)
+            ),
+            // Units as text case
+            new TestCaseSupplier(
+                "DateDiff(" + unit + "<TEXT>, " + startTimestamp + ", " + endTimestamp + ") -> warning ",
+                List.of(DataType.TEXT, DataType.DATETIME, DataType.DATETIME),
+                () -> new TestCaseSupplier.TestCase(
+                    List.of(
+                        new TestCaseSupplier.TypedData(new BytesRef(unit), DataType.TEXT, "unit"),
+                        new TestCaseSupplier.TypedData(startTimestamp.toEpochMilli(), DataType.DATETIME, "startTimestamp"),
+                        new TestCaseSupplier.TypedData(endTimestamp.toEpochMilli(), DataType.DATETIME, "endTimestamp")
+                    ),
+                    "DateDiffEvaluator[unit=Attribute[channel=0], startTimestamp=Attribute[channel=1], "
+                        + "endTimestamp=Attribute[channel=2]]",
+                    DataType.INTEGER,
+                    equalTo(null)
+                ).withWarning("Line -1:-1: evaluation of [] failed, treating result as null. Only first 20 failures recorded.")
+                    .withWarning(warning)
             )
         );
     }