diff --git a/docs/changelog/127739.yaml b/docs/changelog/127739.yaml new file mode 100644 index 0000000000000..e88ff1a6b51c1 --- /dev/null +++ b/docs/changelog/127739.yaml @@ -0,0 +1,7 @@ +pr: 127739 +summary: Fix inconsistent roundup behavior for date ranges using Temporal object as + terms +area: Infra/Core +type: bug +issues: + - 86284 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 0cec1bb9b6a4a..1ad65874d1872 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/RangeType.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/RangeType.java @@ -37,11 +37,13 @@ import java.time.ZoneId; import java.time.ZoneOffset; import java.time.ZonedDateTime; +import java.time.temporal.Temporal; import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Set; import java.util.function.BiFunction; +import java.util.function.LongSupplier; /** Enum defining the type of range */ public enum RangeType { @@ -293,28 +295,27 @@ public Query rangeQuery( DateMathParser dateMathParser = (parser == null) ? DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER.toDateMathParser() : parser; boolean roundUp = includeLower == false; // using "gt" should round lower bound up - Long low = lowerTerm == null - ? minValue() - : dateMathParser.parse( - lowerTerm instanceof BytesRef ? ((BytesRef) lowerTerm).utf8ToString() : lowerTerm.toString(), - context::nowInMillis, - roundUp, - zone - ).toEpochMilli(); + Long low = lowerTerm == null ? minValue() : parseRangeTerm(dateMathParser, lowerTerm, zone, roundUp, context::nowInMillis); roundUp = includeUpper; // using "lte" should round upper bound up - Long high = upperTerm == null - ? maxValue() - : dateMathParser.parse( - upperTerm instanceof BytesRef ? ((BytesRef) upperTerm).utf8ToString() : upperTerm.toString(), - context::nowInMillis, - roundUp, - zone - ).toEpochMilli(); - + Long high = upperTerm == null ? maxValue() : parseRangeTerm(dateMathParser, upperTerm, zone, roundUp, context::nowInMillis); return createRangeQuery(field, hasDocValues, low, high, includeLower, includeUpper, relation); } + private static long parseRangeTerm(DateMathParser parser, Object term, ZoneId zone, boolean roundUp, LongSupplier nowInMillis) { + String termString; + if (term instanceof BytesRef bytesRefTerm) { + termString = bytesRefTerm.utf8ToString(); + } else { + termString = term.toString(); + // `roundUp` may only be used if the term is not a Temporal object. LocalTime#toString() and similar will output + // the shortest possible representation and might omit seconds, milliseconds, etc. + // That interferes badly with the behavior of the roundup parser leading to unexpected results. + roundUp = roundUp && (term instanceof Temporal) == false; + } + return parser.parse(termString, nowInMillis, roundUp, zone).toEpochMilli(); + } + @Override public Query withinQuery(String field, Object from, Object to, boolean includeLower, boolean includeUpper) { return LONG.withinQuery(field, from, to, includeLower, includeUpper); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldTypeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldTypeTests.java index 1a5f3fbcda6bb..ec4f4fe427a2e 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldTypeTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/RangeFieldTypeTests.java @@ -86,10 +86,43 @@ public void testRangeQuery() throws Exception { ); } + public void testDateRangeQueryDoesNotRoundupTemporals() { + type = RangeType.DATE; + SearchExecutionContext context = createContext(); + RangeFieldType ft = createDefaultFieldType(); + ShapeRelation relation = randomFrom(ShapeRelation.values()); + + // explicitly not using nextFrom here to provide a concrete, visual example. + // see the behavioral difference compared to testRangeQueryIntersectsAdjacentDates + ZonedDateTime from = ZonedDateTime.parse("2025-05-01T14:10:00.000Z"); + ZonedDateTime to = ZonedDateTime.parse("2025-05-01T14:11:00.000Z"); + + // usage of roundUp parsers is wrong if terms are Temporal objects, otherwise it would arbitrarily change the bounds + // depending on the string representation + assertEquals( + getExpectedRangeQuery(relation, from, to, false, true), + ft.rangeQuery(from, to, false, true, relation, null, null, context) + ); + } + + public void testRangeQueryIntersectsAdjacentDates() { + type = RangeType.DATE; + SearchExecutionContext context = createContext(); + ShapeRelation relation = randomFrom(ShapeRelation.values()); + RangeFieldType ft = createDefaultFieldType(); + + // explicitly not using nextFrom here to provide a concrete, visual example of the roundUp behavior + String from = "2025-05-01T14:10Z"; // transformed to 2025-05-01T14:10:59.999999999Z by roundUp parser + String to = "2025-05-01T14:11Z"; + + Query rangeQuery = ft.rangeQuery(from, to, false, false, relation, null, null, context); + assertThat(rangeQuery, instanceOf(IndexOrDocValuesQuery.class)); + assertThat(((IndexOrDocValuesQuery) rangeQuery).getIndexQuery(), instanceOf(MatchNoDocsQuery.class)); + } + /** * test the queries are correct if from/to are adjacent and the range is exclusive of those values */ - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/86284") public void testRangeQueryIntersectsAdjacentValues() throws Exception { SearchExecutionContext context = createContext(); ShapeRelation relation = randomFrom(ShapeRelation.values()); @@ -143,7 +176,6 @@ public void testRangeQueryIntersectsAdjacentValues() throws Exception { /** * check that we catch cases where the user specifies larger "from" than "to" value, not counting the include upper/lower settings */ - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/86284") public void testFromLargerToErrors() throws Exception { SearchExecutionContext context = createContext(); RangeFieldType ft = createDefaultFieldType();