From 1897ba7130b754cd722d03f009ccb5e3a09e5d41 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Fri, 28 Mar 2025 11:10:00 -0400 Subject: [PATCH 1/2] 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. --- .../src/main/resources/range.csv-spec | 58 +++++++++++++++++++ .../esql/expression/predicate/Range.java | 15 +++++ .../rules/logical/PropagateEquals.java | 5 ++ 3 files changed, 78 insertions(+) create mode 100644 x-pack/plugin/esql/qa/testFixtures/src/main/resources/range.csv-spec diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/range.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/range.csv-spec new file mode 100644 index 0000000000000..064715b1f148e --- /dev/null +++ b/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 +; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/Range.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/Range.java index fd03165617ab6..118fbe0a7a2aa 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/Range.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/Range.java @@ -115,6 +115,7 @@ public ZoneId zoneId() { */ @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 boolean foldable() { @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 Object fold(FoldContext ctx) { * 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) { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropagateEquals.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropagateEquals.java index 48542a94d1355..8289db1531ddc 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropagateEquals.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropagateEquals.java @@ -256,6 +256,11 @@ private static Expression propagate(Or or, LogicalOptimizerContext ctx) { } // 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())) { From dce5f79cedfb5d20e45066d22aff859b61b2d368 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Mon, 31 Mar 2025 15:29:49 -0400 Subject: [PATCH 2/2] add capability checks --- .../esql/qa/testFixtures/src/main/resources/range.csv-spec | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/range.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/range.csv-spec index 064715b1f148e..39958394eab16 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/range.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/range.csv-spec @@ -29,6 +29,8 @@ Invalid Boundaries, date nanos required_capability: date_nanos_type required_capability: to_date_nanos required_capability: date_nanos_binary_comparison +required_capability: fix_date_nanos_lucene_pushdown_bug +required_capability: fix_date_nanos_mixed_range_pushdown_bug FROM date_nanos | WHERE nanos > TO_DATE_NANOS("2025-01-01") and nanos < TO_DATE_NANOS("2020-01-01") @@ -45,6 +47,8 @@ Invalid Boundaries, date nanos implicit casting required_capability: date_nanos_type required_capability: date_nanos_binary_comparison required_capability: date_nanos_implicit_casting +required_capability: fix_date_nanos_lucene_pushdown_bug +required_capability: fix_date_nanos_mixed_range_pushdown_bug FROM date_nanos | WHERE nanos > "2025-01-01" and nanos < "2020-01-01"