[ES|QL] Create Range in PushFiltersToSource for qualified pushable fi…

…lters on the same field (elastic#111437)
cbuescher · Sep 4, 2024 · e1f220e · e1f220e
1 parent 1e8ac2f
commit e1f220e
Show file tree

Hide file tree

Showing 10 changed files with 439 additions and 107 deletions.
diff --git a/docs/changelog/110548.yaml b/docs/changelog/110548.yaml
diff --git a/docs/changelog/111437.yaml b/docs/changelog/111437.yaml
@@ -0,0 +1,5 @@
+pr: 111437
+summary: "[ES|QL] Create `Range` in `PushFiltersToSource` for qualified pushable filters on the same field"
+area: ES|QL
+type: enhancement
+issues: []
diff --git a/...l-core/src/main/java/org/elasticsearch/xpack/esql/core/planner/ExpressionTranslators.java b/...l-core/src/main/java/org/elasticsearch/xpack/esql/core/planner/ExpressionTranslators.java
@@ -7,12 +7,10 @@
 
 package org.elasticsearch.xpack.esql.core.planner;
 
-import org.elasticsearch.common.time.DateFormatter;
 import org.elasticsearch.xpack.esql.core.QlIllegalArgumentException;
 import org.elasticsearch.xpack.esql.core.expression.Expression;
 import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
 import org.elasticsearch.xpack.esql.core.expression.MetadataAttribute;
-import org.elasticsearch.xpack.esql.core.expression.predicate.Range;
 import org.elasticsearch.xpack.esql.core.expression.predicate.fulltext.MatchQueryPredicate;
 import org.elasticsearch.xpack.esql.core.expression.predicate.fulltext.MultiMatchQueryPredicate;
 import org.elasticsearch.xpack.esql.core.expression.predicate.fulltext.StringQueryPredicate;
@@ -32,31 +30,17 @@
 import org.elasticsearch.xpack.esql.core.querydsl.query.NotQuery;
 import org.elasticsearch.xpack.esql.core.querydsl.query.Query;
 import org.elasticsearch.xpack.esql.core.querydsl.query.QueryStringQuery;
-import org.elasticsearch.xpack.esql.core.querydsl.query.RangeQuery;
 import org.elasticsearch.xpack.esql.core.querydsl.query.RegexQuery;
 import org.elasticsearch.xpack.esql.core.querydsl.query.WildcardQuery;
 import org.elasticsearch.xpack.esql.core.tree.Source;
 import org.elasticsearch.xpack.esql.core.util.Check;
 import org.elasticsearch.xpack.esql.core.util.CollectionUtils;
 
-import java.time.OffsetTime;
-import java.time.ZonedDateTime;
-import java.time.temporal.TemporalAccessor;
 import java.util.Arrays;
 import java.util.List;
 
 public final class ExpressionTranslators {
 
-    public static final String DATE_FORMAT = "strict_date_optional_time_nanos";
-    public static final String TIME_FORMAT = "strict_hour_minute_second_fraction";
-
-    public static Object valueOf(Expression e) {
-        if (e.foldable()) {
-            return e.fold();
-        }
-        throw new QlIllegalArgumentException("Cannot determine value for {}", e);
-    }
-
     // TODO: see whether escaping is needed
     @SuppressWarnings("rawtypes")
     public static class Likes extends ExpressionTranslator<RegexMatch> {
@@ -193,55 +177,6 @@ private static Query translate(IsNull isNull, TranslatorHandler handler) {
         }
     }
 
-    public static class Ranges extends ExpressionTranslator<Range> {
-
-        @Override
-        protected Query asQuery(Range r, TranslatorHandler handler) {
-            return doTranslate(r, handler);
-        }
-
-        public static Query doTranslate(Range r, TranslatorHandler handler) {
-            return handler.wrapFunctionQuery(r, r.value(), () -> translate(r, handler));
-        }
-
-        private static RangeQuery translate(Range r, TranslatorHandler handler) {
-            Object lower = valueOf(r.lower());
-            Object upper = valueOf(r.upper());
-            String format = null;
-
-            // for a date constant comparison, we need to use a format for the date, to make sure that the format is the same
-            // no matter the timezone provided by the user
-            DateFormatter formatter = null;
-            if (lower instanceof ZonedDateTime || upper instanceof ZonedDateTime) {
-                formatter = DateFormatter.forPattern(DATE_FORMAT);
-            } else if (lower instanceof OffsetTime || upper instanceof OffsetTime) {
-                formatter = DateFormatter.forPattern(TIME_FORMAT);
-            }
-            if (formatter != null) {
-                // RangeQueryBuilder accepts an Object as its parameter, but it will call .toString() on the ZonedDateTime
-                // instance which can have a slightly different format depending on the ZoneId used to create the ZonedDateTime
-                // Since RangeQueryBuilder can handle date as String as well, we'll format it as String and provide the format.
-                if (lower instanceof ZonedDateTime || lower instanceof OffsetTime) {
-                    lower = formatter.format((TemporalAccessor) lower);
-                }
-                if (upper instanceof ZonedDateTime || upper instanceof OffsetTime) {
-                    upper = formatter.format((TemporalAccessor) upper);
-                }
-                format = formatter.pattern();
-            }
-            return new RangeQuery(
-                r.source(),
-                handler.nameOf(r.value()),
-                lower,
-                r.includeLower(),
-                upper,
-                r.includeUpper(),
-                format,
-                r.zoneId()
-            );
-        }
-    }
-
     public static Query or(Source source, Query left, Query right) {
         return boolQuery(source, left, right, false);
     }

diff --git a/...n/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java b/...n/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java
@@ -62,6 +62,7 @@
 import static org.elasticsearch.xpack.esql.EsqlTestUtils.as;
 import static org.elasticsearch.xpack.esql.qa.rest.RestEsqlTestCase.Mode.ASYNC;
 import static org.elasticsearch.xpack.esql.qa.rest.RestEsqlTestCase.Mode.SYNC;
+import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.dateTimeToString;
 import static org.hamcrest.Matchers.any;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.either;
@@ -440,6 +441,65 @@ public void testOutOfRangeComparisons() throws IOException {
         }
     }
 
+    // Test the Range created in PushFiltersToSource for qualified pushable filters on the same field
+    public void testInternalRange() throws IOException {
+        final int NUM_SINGLE_VALUE_ROWS = 100;
+        bulkLoadTestData(NUM_SINGLE_VALUE_ROWS);
+        bulkLoadTestData(10, NUM_SINGLE_VALUE_ROWS, false, RestEsqlTestCase::createDocumentWithMVs);
+        bulkLoadTestData(5, NUM_SINGLE_VALUE_ROWS + 10, false, RestEsqlTestCase::createDocumentWithNulls);
+
+        String upperBound = randomFrom(" < ", " <= ");
+        String lowerBound = randomFrom(" > ", " >= ");
+
+        String predicate = "{}" + upperBound + "{} and {}" + lowerBound + "{} and {} != {}";
+        int half = NUM_SINGLE_VALUE_ROWS / 2;
+        int halfPlusThree = half + 3;
+        List<String> predicates = List.of(
+            format(null, predicate, "integer", half, "integer", -1, "integer", half),
+            format(null, predicate, "short", half, "short", -1, "short", half),
+            format(null, predicate, "byte", half, "byte", -1, "byte", half),
+            format(null, predicate, "long", half, "long", -1, "long", half),
+            format(null, predicate, "double", half, "double", -1.0, "double", half),
+            format(null, predicate, "float", half, "float", -1.0, "float", half),
+            format(null, predicate, "half_float", half, "half_float", -1.0, "half_float", half),
+            format(null, predicate, "scaled_float", half, "scaled_float", -1.0, "scaled_float", half),
+            format(
+                null,
+                predicate,
+                "date",
+                "\"" + dateTimeToString(half) + "\"",
+                "date",
+                "\"1001-01-01\"",
+                "date",
+                "\"" + dateTimeToString(half) + "\""
+            ),
+            // keyword6-9 is greater than keyword53, [54,99] + [6, 9], 50 items in total
+            format(
+                null,
+                predicate,
+                "keyword",
+                "\"keyword999\"",
+                "keyword",
+                "\"keyword" + halfPlusThree + "\"",
+                "keyword",
+                "\"keyword" + halfPlusThree + "\""
+            ),
+            format(null, predicate, "ip", "\"127.0.0." + half + "\"", "ip", "\"126.0.0.0\"", "ip", "\"127.0.0." + half + "\""),
+            format(null, predicate, "version", "\"1.2." + half + "\"", "version", "\"1.2\"", "version", "\"1.2." + half + "\"")
+        );
+
+        for (String p : predicates) {
+            var query = requestObjectBuilder().query(format(null, "from {} | where {}", testIndexName(), p));
+            var result = runEsql(query, List.of(), NO_WARNINGS_REGEX, mode);
+            var values = as(result.get("values"), ArrayList.class);
+            assertThat(
+                format(null, "Comparison [{}] should return all rows with single values.", p),
+                values.size(),
+                is(NUM_SINGLE_VALUE_ROWS / 2)
+            );
+        }
+    }
+
     public void testWarningHeadersOnFailedConversions() throws IOException {
         int count = randomFrom(10, 40, 60);
         bulkLoadTestData(count);

diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/comparison.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/comparison.csv-spec
@@ -116,3 +116,99 @@ from employees
 emp_no:integer
 10001
 ;
+
+rangeInteger
+from employees
+| where emp_no > 10001 and emp_no < 10003
+| keep emp_no, first_name
+;
+
+emp_no:integer |first_name:keyword
+10002          |Bezalel
+;
+
+rangeLong
+from employees
+| where languages.long > 1 and languages.long < 3
+| keep emp_no, first_name
+| sort emp_no
+| limit 2
+;
+
+emp_no:integer |first_name:keyword
+10001          |Georgi
+10008          |Saniya
+;
+
+rangeDouble
+from employees
+| where height > 1.4 and height < 2.0
+| keep emp_no, first_name
+| sort emp_no
+| limit 2
+;
+
+emp_no:integer |first_name:keyword
+10003          |Parto
+10004          |Chirstian
+;
+
+rangeUnsignedLong
+from ul_logs
+| where bytes_out >= to_ul(4747671420480199905) and bytes_out <= to_ul(12749081495402663265)
+| keep id
+| sort id
+| limit 2
+;
+
+id:integer
+1
+3
+;
+
+
+rangeKeyword
+from employees
+| where first_name >= "A" and first_name <= "D"
+| keep emp_no, first_name
+| sort emp_no
+| limit 2
+;
+
+emp_no:integer |first_name:keyword
+10002          |Bezalel
+10004          |Chirstian
+;
+
+rangeVersion
+from apps
+| where version > "2" and version < "4"
+| keep id, version
+| sort id
+;
+
+id:integer |version:version
+2          |2.1
+3          |2.3.4
+4          |2.12.0
+;
+
+rangeDateTime
+from employees
+| where birth_date >= "1952-01-01" and birth_date <= "1952-12-31"
+| stats cnt = count(*)
+;
+
+cnt:long
+8
+;
+
+rangeMixed
+from employees
+| where birth_date >= "1952-01-01" and birth_date <= "1952-12-31" and hire_date >= "1980-01-01" and hire_date <= "1989-12-31"
+| stats cnt = count(*)
+;
+
+cnt:long
+5
+;
diff --git a/.../org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialRelatesUtils.java b/.../org/elasticsearch/xpack/esql/expression/function/scalar/spatial/SpatialRelatesUtils.java
@@ -28,7 +28,7 @@
 
 import java.io.IOException;
 
-import static org.elasticsearch.xpack.esql.core.planner.ExpressionTranslators.valueOf;
+import static org.elasticsearch.xpack.esql.core.expression.Foldables.valueOf;
 
 public class SpatialRelatesUtils {