From 3ae341c187a688cb22d2f20442733b1c2f29e3d8 Mon Sep 17 00:00:00 2001 From: john-wagster Date: Tue, 1 Oct 2024 10:40:55 -0500 Subject: [PATCH] updated rangetype to be more inline with the docs (https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-range-query.html) and added tests to reflect as much (#113872) --- docs/reference/mapping/types/range.asciidoc | 2 +- .../rest-api-spec/test/range/10_basic.yml | 71 +++++++++ .../test/range/20_synthetic_source.yml | 137 ++++++++++++++++++ .../index/mapper/MapperFeatures.java | 5 + .../index/mapper/RangeFieldMapper.java | 1 + .../elasticsearch/index/mapper/RangeType.java | 11 +- .../mapper/DateRangeFieldMapperTests.java | 2 +- 7 files changed, 225 insertions(+), 4 deletions(-) diff --git a/docs/reference/mapping/types/range.asciidoc b/docs/reference/mapping/types/range.asciidoc index fa52722c4c7d1..14c5b6098acbe 100644 --- 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 index c92d75cf2f5cb..3432a1e34c018 100644 --- 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 index 07bd372b60058..cc92b52e0887a 100644 --- 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 index 6d24707ba0058..eb424117ecae2 100644 --- 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 getFeatures() { SourceFieldMapper.SYNTHETIC_SOURCE_STORED_FIELDS_ADVANCE_FIX ); } + + @Override + public Set 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 index e80da7ce8a763..b814c25360e82 100644 --- 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 index bd307445c9717..c916d67789e60 100644 --- 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 index c2b1abca6cbc6..f669096efa3d8 100644 --- 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