Bladeren bron

ESQL - CSV tests for invalid ranges (#125714)

Add CSV tests for invalid ranges (i.e. where the lower bound is greater than the upper bound) for several data types. It looks at first glance like there should be a bug here with Date Nanos, but I cannot trigger one, and analysis suggests the code that has the bug is in fact unreachable. I've left comments to that effect.
Mark Tozzi 6 maanden geleden
bovenliggende
commit
27ebb14722

+ 58 - 0
x-pack/plugin/esql/qa/testFixtures/src/main/resources/range.csv-spec

@@ -0,0 +1,58 @@
+Invalid boundaries integer literal
+
+FROM employees 
+| WHERE salary > 200 AND salary < 100 
+| KEEP salary;
+
+salary:integer
+;
+
+Invalid boundaries date 
+
+FROM employees
+| WHERE hire_date > TO_DATETIME("2025-01-01") AND hire_date < TO_DATETIME("2020-01-01")
+| KEEP hire_date;
+
+hire_date:datetime
+;
+
+Invalid boundaries, date with implicit casting
+
+FROM employees
+| WHERE hire_date > "2025-01-01" AND hire_date < "2020-01-01"
+| KEEP hire_date;
+
+hire_date:datetime
+;
+
+Invalid Boundaries, date nanos 
+required_capability: date_nanos_type
+required_capability: to_date_nanos
+required_capability: date_nanos_binary_comparison
+
+FROM date_nanos
+| WHERE nanos > TO_DATE_NANOS("2025-01-01") and nanos < TO_DATE_NANOS("2020-01-01")
+| KEEP nanos;
+warningRegex:Line 2:49: evaluation of \[nanos < TO_DATE_NANOS\(\\\"2020-01-01\\\"\)\] failed, treating result as null\. Only first 20 failures recorded\.
+warningRegex:Line 2:49: java\.lang\.IllegalArgumentException: single-value function encountered multi-value
+warningRegex:Line 2:9: evaluation of \[nanos > TO_DATE_NANOS\(\\\"2025-01-01\\\"\)\] failed, treating result as null\. Only first 20 failures recorded\.
+warningRegex:Line 2:9: java\.lang\.IllegalArgumentException: single-value function encountered multi-value
+
+nanos:date_nanos
+;
+
+Invalid Boundaries, date nanos implicit casting
+required_capability: date_nanos_type
+required_capability: date_nanos_binary_comparison
+required_capability: date_nanos_implicit_casting
+
+FROM date_nanos
+| WHERE nanos > "2025-01-01" and nanos < "2020-01-01"
+| KEEP nanos;
+warningRegex:Line 2:34: evaluation of \[nanos < \\\"2020-01-01\\\"\] failed, treating result as null\. Only first 20 failures recorded\.
+warningRegex:Line 2:34: java\.lang\.IllegalArgumentException: single-value function encountered multi-value
+warningRegex:Line 2:9: evaluation of \[nanos > \\\"2025-01-01\\\"\] failed, treating result as null\. Only first 20 failures recorded\.
+warningRegex:Line 2:9: java\.lang\.IllegalArgumentException: single-value function encountered multi-value
+
+nanos:date_nanos
+;

+ 15 - 0
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/Range.java

@@ -115,6 +115,7 @@ public class Range extends ScalarFunction implements TranslationAware.SingleValu
      */
     @Override
     public boolean foldable() {
+        // NB: this is likely dead code. See note in areBoundariesInvalid
         if (lower.foldable() && upper.foldable()) {
             if (value().foldable()) {
                 return true;
@@ -130,6 +131,7 @@ public class Range extends ScalarFunction implements TranslationAware.SingleValu
 
     @Override
     public Object fold(FoldContext ctx) {
+        // NB: this is likely dead code. See note in areBoundariesInvalid
         Object lowerValue = lower.fold(ctx);
         Object upperValue = upper.fold(ctx);
         if (areBoundariesInvalid(lowerValue, upperValue)) {
@@ -149,6 +151,19 @@ public class Range extends ScalarFunction implements TranslationAware.SingleValu
      * If they are, the value does not have to be evaluated.
      */
     protected boolean areBoundariesInvalid(Object lowerValue, Object upperValue) {
+        /*
+        NB: I am reasonably sure this code is dead.  It can only be reached from foldable(), and as far as I can tell
+        we never fold ranges. There's no ES|QL syntax for ranges, so they can never be created by the parser.  The
+        PropagateEquals optimizer rule can in theory create ranges, but only from existing ranges.  The fact that this
+        class is not serializable (note that writeTo throws UnsupportedOperationException) is a clear indicator that
+        logical planning cannot output Range nodes.
+
+        PushFiltersToSource can also create ranges, but that is a Physical optimizer rule. Folding happens in the
+        Logical optimization layer, and should be done by the time we are calling PushFiltersToSource.
+
+        That said, if somehow you have arrived here while debugging something, know that this method is likely
+        completely broken for date_nanos, and possibly other types.
+         */
         if (DataType.isDateTime(value.dataType()) || DataType.isDateTime(lower.dataType()) || DataType.isDateTime(upper.dataType())) {
             try {
                 if (upperValue instanceof String upperString) {

+ 5 - 0
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropagateEquals.java

@@ -256,6 +256,11 @@ public final class PropagateEquals extends OptimizerRules.OptimizerExpressionRul
             }
 
             // Equals OR Range
+            /*
+            NB: this loop is probably dead code. There's no syntax for ranges, so the parser never produces them.  This
+            rule can create ranges, but only in this loop, which iterates over the existing ranges. In short,
+            ranges.size() should always be zero at this point.
+             */
             for (int i = 0; i < ranges.size(); i++) { // might modify list, so use index loop
                 Range range = ranges.get(i);
                 if (eq.left().semanticEquals(range.value())) {