updated rangetype to be more inline with the docs (https://www.elasti…

…c.co/guide/en/elasticsearch/reference/current/query-dsl-range-query.html) and added tests to reflect as much (elastic#113872)
john-wagster · Oct 1, 2024 · 3ae341c · 3ae341c
1 parent 075ff37
commit 3ae341c
Show file tree

Hide file tree

Showing 7 changed files with 225 additions and 4 deletions.
diff --git a/docs/reference/mapping/types/range.asciidoc b/docs/reference/mapping/types/range.asciidoc
@@ -465,7 +465,7 @@ Will become:
     },
     {
       "gte": "2017-09-01T00:00:00.000Z",
-      "lte": "2017-09-10T00:00:00.000Z"
+      "lte": "2017-09-10T23:59:59.999Z"
     }
   ]
 }

diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/range/10_basic.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/range/10_basic.yml
@@ -446,6 +446,77 @@ setup:
 
   - match: { hits.total: 2 }
 
+---
+"Date range edge cases":
+  - requires:
+      cluster_features: ["mapper.range.date_range_indexing_fix"]
+      reason: "tests rounding fixes in 8.16.0 that previously caused non-intuitive indexing and query because ranges were assumed to always index with 0's as the default such as when time is missing 00:00:00.000 time was assumed but for lte indexing and query missing time should be 23:59:59.999 as per docs here: https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-range-query.html"
+
+  - do:
+      indices.create:
+        index: test_date_range_edge_cases
+        body:
+          mappings:
+            "properties":
+              "date_range":
+                "type": "date_range"
+                "format": "yyyy-MM-dd"
+
+  - do:
+      index:
+        index: test_date_range_edge_cases
+        id: "1"
+        body: { "date_range": { "gte": "1980-12-14", "lte": "1980-12-17" } }
+
+  - do:
+      index:
+        index: test_date_range_edge_cases
+        id: "2"
+        body: { "date_range": { "gt": "1990-12-15", "lt": "1990-12-18" } }
+
+  - do:
+      index:
+        index: test_date_range_edge_cases
+        id: "3"
+        body: { "date_range": { "gte": "1985-12-16||/M", "lte": "1986-02-10||/M" } }
+
+  - do:
+      index:
+        index: test_date_range_edge_cases
+        id: "4"
+        body: { "date_range": { "gt": "1995-12-16||/M", "lt": "1996-02-10||/M" } }
+
+  - do:
+      indices.refresh: { }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        body: { "size": 0, "query": { "range": { "date_range": { "gte": "1980-12-14", "lte": "1980-12-17", "relation": "contains" } } } }
+
+  - match: { hits.total: 1 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        body: { "size": 0, "query": { "range": { "date_range": { "gt": "1990-12-15", "lt": "1990-12-18", "relation": "contains" } } } }
+
+  - match: { hits.total: 1 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        body: { "size": 0, "query": { "range": { "date_range": { "gte": "1985-12-16||/M", "lte": "1986-02-10||/M", "relation": "contains" } } } }
+
+  - match: { hits.total: 1 }
+
+  - do:
+      search:
+        rest_total_hits_as_int: true
+        body: { "size": 0, "query": { "range": { "date_range": { "gt": "1995-12-16||/M", "lt": "1996-02-10||/M", "relation": "contains" } } } }
+
+  - match: { hits.total: 1 }
+
 ---
 "Null bounds":
 

diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/range/20_synthetic_source.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/range/20_synthetic_source.yml
@@ -527,6 +527,9 @@ setup:
 
 ---
 "Date range":
+  - skip:
+      cluster_features: ["mapper.range.date_range_indexing_fix"]
+      reason: "tests prior to rounding fixes in 8.16.0 that caused non-intuitive indexing and query because ranges were assumed to always index with 0's as the default such as when time is missing 00:00:00.000 time was assumed but for lte indexing and query missing time should be 23:59:59.999 as per docs here: https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-range-query.html"
 
   - do:
       index:
@@ -655,3 +658,137 @@ setup:
   - match:
       _source:
         date_range: { "gte": "2017-09-05T00:00:00.000Z", "lte": null }
+
+---
+"Date range Rounding Fixes":
+  - requires:
+      cluster_features: ["mapper.range.date_range_indexing_fix"]
+      reason: "tests rounding fixes in 8.16.0 that previously caused non-intuitive indexing and query because ranges were assumed to always index with 0's as the default such as when time is missing 00:00:00.000 time was assumed but for lte indexing and query missing time should be 23:59:59.999 as per docs here: https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-range-query.html"
+
+  - do:
+      index:
+        index: synthetic_source_test
+        id: "1"
+        body: { "date_range": { "gte": "2017-09-01", "lte": "2017-09-05" } }
+
+  - do:
+      index:
+        index: synthetic_source_test
+        id: "2"
+        body: { "date_range": { "gt": "2017-09-01", "lte": "2017-09-03" } }
+
+  - do:
+      index:
+        index: synthetic_source_test
+        id: "3"
+        body: { "date_range": [ { "gte": "2017-09-04", "lt": "2017-09-05" } ] }
+
+  - do:
+      index:
+        index: synthetic_source_test
+        id: "4"
+        body: { "date_range": [ { "gt": "2017-09-04", "lt": "2017-09-08" }, { "gt": "2017-09-04", "lt": "2017-09-07" } ] }
+
+  - do:
+      index:
+        index: synthetic_source_test
+        id: "5"
+        body: { "date_range": { "gte": 1504224000000, "lte": 1504569600000 } }
+
+  - do:
+      index:
+        index: synthetic_source_test
+        id: "6"
+        body: { "date_range": { "gte": "2017-09-01T10:20:30.123Z", "lte": "2017-09-05T03:04:05.789Z" } }
+
+  - do:
+      index:
+        index: synthetic_source_test
+        id: "7"
+        body: { "date_range": null }
+
+  - do:
+      index:
+        index: synthetic_source_test
+        id: "8"
+        body: { "date_range": { "gte": null, "lte": "2017-09-05" } }
+
+  - do:
+      index:
+        index: synthetic_source_test
+        id: "9"
+        body: { "date_range": { "gte": "2017-09-05" } }
+
+  - do:
+      indices.refresh: { }
+
+  - do:
+      get:
+        index: synthetic_source_test
+        id: "1"
+  - match:
+      _source:
+        date_range: { "gte": "2017-09-01T00:00:00.000Z", "lte": "2017-09-05T23:59:59.999Z" }
+
+  - do:
+      get:
+        index: synthetic_source_test
+        id: "2"
+  - match:
+      _source:
+        date_range: { "gte": "2017-09-02T00:00:00.000Z", "lte": "2017-09-03T23:59:59.999Z" }
+
+  - do:
+      get:
+        index: synthetic_source_test
+        id: "3"
+  - match:
+      _source:
+        date_range: { "gte": "2017-09-04T00:00:00.000Z", "lte": "2017-09-04T23:59:59.999Z" }
+
+  - do:
+      get:
+        index: synthetic_source_test
+        id: "4"
+  - match:
+      _source:
+        date_range: [ { "gte": "2017-09-05T00:00:00.000Z", "lte": "2017-09-06T23:59:59.999Z" }, { "gte": "2017-09-05T00:00:00.000Z", "lte": "2017-09-07T23:59:59.999Z" } ]
+
+  - do:
+      get:
+        index: synthetic_source_test
+        id: "5"
+  - match:
+      _source:
+        date_range: { "gte": "2017-09-01T00:00:00.000Z", "lte": "2017-09-05T00:00:00.000Z" }
+
+  - do:
+      get:
+        index: synthetic_source_test
+        id: "6"
+  - match:
+      _source:
+        date_range: { "gte": "2017-09-01T10:20:30.123Z", "lte": "2017-09-05T03:04:05.789Z" }
+
+  - do:
+      get:
+        index: synthetic_source_test
+        id: "7"
+  - match:
+      _source: { }
+
+  - do:
+      get:
+        index: synthetic_source_test
+        id: "8"
+  - match:
+      _source:
+        date_range: { "gte": null, "lte": "2017-09-05T23:59:59.999Z" }
+
+  - do:
+      get:
+        index: synthetic_source_test
+        id: "9"
+  - match:
+      _source:
+        date_range: { "gte": "2017-09-05T00:00:00.000Z", "lte": null }
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java
@@ -32,4 +32,9 @@ public Set<NodeFeature> getFeatures() {
             SourceFieldMapper.SYNTHETIC_SOURCE_STORED_FIELDS_ADVANCE_FIX
         );
     }
+
+    @Override
+    public Set<NodeFeature> getTestFeatures() {
+        return Set.of(RangeFieldMapper.DATE_RANGE_INDEXING_FIX);
+    }
 }
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java
@@ -51,6 +51,7 @@
 /** A {@link FieldMapper} for indexing numeric and date ranges, and creating queries */
 public class RangeFieldMapper extends FieldMapper {
     public static final NodeFeature NULL_VALUES_OFF_BY_ONE_FIX = new NodeFeature("mapper.range.null_values_off_by_one_fix");
+    public static final NodeFeature DATE_RANGE_INDEXING_FIX = new NodeFeature("mapper.range.date_range_indexing_fix");
 
     public static final boolean DEFAULT_INCLUDE_UPPER = true;
     public static final boolean DEFAULT_INCLUDE_LOWER = true;

diff --git a/server/src/main/java/org/elasticsearch/index/mapper/RangeType.java b/server/src/main/java/org/elasticsearch/index/mapper/RangeType.java
@@ -195,14 +195,20 @@ public Field getRangeField(String name, RangeFieldMapper.Range r) {
         @Override
         public Number parseFrom(RangeFieldMapper.RangeFieldType fieldType, XContentParser parser, boolean coerce, boolean included)
             throws IOException {
-            Number value = parseValue(parser.text(), coerce, fieldType.dateMathParser);
+            assert fieldType.dateMathParser != null;
+            Number value = fieldType.dateMathParser.parse(parser.text(), () -> {
+                throw new IllegalArgumentException("now is not used at indexing time");
+            }, included == false, null).toEpochMilli();
             return included ? value : nextUp(value);
         }
 
         @Override
         public Number parseTo(RangeFieldMapper.RangeFieldType fieldType, XContentParser parser, boolean coerce, boolean included)
             throws IOException {
-            Number value = parseValue(parser.text(), coerce, fieldType.dateMathParser);
+            assert fieldType.dateMathParser != null;
+            Number value = fieldType.dateMathParser.parse(parser.text(), () -> {
+                throw new IllegalArgumentException("now is not used at indexing time");
+            }, included, null).toEpochMilli();
             return included ? value : nextDown(value);
         }
 
@@ -294,6 +300,7 @@ public Query rangeQuery(
                     roundUp,
                     zone
                 ).toEpochMilli();
+
             roundUp = includeUpper; // using "lte" should round upper bound up
             Long high = upperTerm == null
                 ? maxValue()

diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DateRangeFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DateRangeFieldMapperTests.java
@@ -32,7 +32,7 @@ protected XContentBuilder rangeSource(XContentBuilder in) throws IOException {
 
     @Override
     protected String storedValue() {
-        return "1477872000000";
+        return "1477958399999";
     }
 
     @Override