Skip to content

Commit 4e23cd0

Browse files
committed
PR feedback
1 parent 9bafb50 commit 4e23cd0

File tree

2 files changed

+45
-6
lines changed

2 files changed

+45
-6
lines changed

server/src/main/java/org/elasticsearch/index/mapper/RangeType.java

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -303,12 +303,17 @@ public Query rangeQuery(
303303
}
304304

305305
private static long parseRangeTerm(DateMathParser parser, Object term, ZoneId zone, boolean roundUp, LongSupplier nowInMillis) {
306-
String termString = term instanceof BytesRef ? ((BytesRef) term).utf8ToString() : term.toString();
307-
// `roundUp` may only be used if the term is not a Temporal object. LocalTime#toString() and similar will output
308-
// the shortest possible representation and might omit seconds, milliseconds, etc.
309-
// That interferes badly with the behavior of the roundup parser leading to unexpected results.
310-
boolean mayRoundUp = (term instanceof Temporal) == false;
311-
return parser.parse(termString, nowInMillis, roundUp && mayRoundUp, zone).toEpochMilli();
306+
String termString;
307+
if (term instanceof BytesRef bytesRefTerm) {
308+
termString = bytesRefTerm.utf8ToString();
309+
} else {
310+
termString = term.toString();
311+
// `roundUp` may only be used if the term is not a Temporal object. LocalTime#toString() and similar will output
312+
// the shortest possible representation and might omit seconds, milliseconds, etc.
313+
// That interferes badly with the behavior of the roundup parser leading to unexpected results.
314+
roundUp = roundUp && (term instanceof Temporal) == false;
315+
}
316+
return parser.parse(termString, nowInMillis, roundUp, zone).toEpochMilli();
312317
}
313318

314319
@Override

server/src/test/java/org/elasticsearch/index/mapper/RangeFieldTypeTests.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,40 @@ public void testRangeQuery() throws Exception {
8686
);
8787
}
8888

89+
public void testDateRangeQueryDoesNotRoundupTemporals() {
90+
type = RangeType.DATE;
91+
SearchExecutionContext context = createContext();
92+
RangeFieldType ft = createDefaultFieldType();
93+
ShapeRelation relation = randomFrom(ShapeRelation.values());
94+
95+
// explicitly not using nextFrom here to provide a concrete, visual example.
96+
// see the behavioral difference compared to testRangeQueryIntersectsAdjacentDates
97+
ZonedDateTime from = ZonedDateTime.parse("2025-05-01T14:10:00.000Z");
98+
ZonedDateTime to = ZonedDateTime.parse("2025-05-01T14:11:00.000Z");
99+
100+
// usage of roundUp parsers is wrong if terms are Temporal objects, otherwise it would arbitrarily change the bounds
101+
// depending on the string representation
102+
assertEquals(
103+
getExpectedRangeQuery(relation, from, to, false, true),
104+
ft.rangeQuery(from, to, false, true, relation, null, null, context)
105+
);
106+
}
107+
108+
public void testRangeQueryIntersectsAdjacentDates() {
109+
type = RangeType.DATE;
110+
SearchExecutionContext context = createContext();
111+
ShapeRelation relation = randomFrom(ShapeRelation.values());
112+
RangeFieldType ft = createDefaultFieldType();
113+
114+
// explicitly not using nextFrom here to provide a concrete, visual example of the roundUp behavior
115+
String from = "2025-05-01T14:10Z"; // transformed to 2025-05-01T14:10:59.999999999Z by roundUp parser
116+
String to = "2025-05-01T14:11Z";
117+
118+
Query rangeQuery = ft.rangeQuery(from, to, false, false, relation, null, null, context);
119+
assertThat(rangeQuery, instanceOf(IndexOrDocValuesQuery.class));
120+
assertThat(((IndexOrDocValuesQuery) rangeQuery).getIndexQuery(), instanceOf(MatchNoDocsQuery.class));
121+
}
122+
89123
/**
90124
* test the queries are correct if from/to are adjacent and the range is exclusive of those values
91125
*/

0 commit comments

Comments
 (0)