Skip to content

Commit ca64596

Browse files
sidoserasidosera
authored
ESQL: Fix BUCKET date rounding selection (#144425)
1 parent cde98aa commit ca64596

File tree

3 files changed

+113
-29
lines changed
  • docs/changelog
  • x-pack/plugin/esql/src
    • main/java/org/elasticsearch/xpack/esql/expression/function/grouping
    • test/java/org/elasticsearch/xpack/esql/expression/function/grouping

3 files changed

+113
-29
lines changed

‎docs/changelog/144425.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
area: ES|QL
2+
issues: []
3+
pr: 144425
4+
summary: Fix BUCKET date rounding selection
5+
type: bug

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/grouping/Bucket.java‎

Lines changed: 63 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -70,30 +70,44 @@ public class Bucket extends GroupingFunction.EvaluatableGroupingFunction
7070

7171
// TODO maybe we should just cover the whole of representable dates here - like ten years, 100 years, 1000 years, all the way up.
7272
// That way you never end up with more than the target number of buckets.
73-
private static final Function<ZoneId, Rounding> LARGEST_HUMAN_DATE_ROUNDING = (zoneId) -> Rounding.builder(
74-
Rounding.DateTimeUnit.YEAR_OF_CENTURY
75-
).timeZone(zoneId).build();
76-
private static final List<Function<ZoneId, Rounding>> HUMAN_DATE_ROUNDINGS = List.of(
77-
(zoneId) -> Rounding.builder(Rounding.DateTimeUnit.MONTH_OF_YEAR).timeZone(zoneId).build(),
78-
(zoneId) -> Rounding.builder(Rounding.DateTimeUnit.WEEK_OF_WEEKYEAR).timeZone(zoneId).build(),
79-
(zoneId) -> Rounding.builder(Rounding.DateTimeUnit.DAY_OF_MONTH).timeZone(zoneId).build(),
80-
(zoneId) -> Rounding.builder(TimeValue.timeValueHours(12)).timeZone(zoneId).build(),
81-
(zoneId) -> Rounding.builder(TimeValue.timeValueHours(3)).timeZone(zoneId).build(),
82-
(zoneId) -> Rounding.builder(TimeValue.timeValueHours(1)).timeZone(zoneId).build(),
83-
(zoneId) -> Rounding.builder(TimeValue.timeValueMinutes(30)).timeZone(zoneId).build(),
84-
(zoneId) -> Rounding.builder(TimeValue.timeValueMinutes(10)).timeZone(zoneId).build(),
85-
(zoneId) -> Rounding.builder(TimeValue.timeValueMinutes(5)).timeZone(zoneId).build(),
86-
(zoneId) -> Rounding.builder(TimeValue.timeValueMinutes(1)).timeZone(zoneId).build(),
87-
(zoneId) -> Rounding.builder(TimeValue.timeValueSeconds(30)).timeZone(zoneId).build(),
88-
(zoneId) -> Rounding.builder(TimeValue.timeValueSeconds(10)).timeZone(zoneId).build(),
89-
(zoneId) -> Rounding.builder(TimeValue.timeValueSeconds(5)).timeZone(zoneId).build(),
90-
(zoneId) -> Rounding.builder(TimeValue.timeValueSeconds(1)).timeZone(zoneId).build(),
91-
(zoneId) -> Rounding.builder(TimeValue.timeValueMillis(100)).timeZone(zoneId).build(),
92-
(zoneId) -> Rounding.builder(TimeValue.timeValueMillis(50)).timeZone(zoneId).build(),
93-
(zoneId) -> Rounding.builder(TimeValue.timeValueMillis(10)).timeZone(zoneId).build(),
94-
(zoneId) -> Rounding.builder(TimeValue.timeValueMillis(1)).timeZone(zoneId).build()
73+
private static final Unit YEAR_OF_CENTURY = Unit.from(Rounding.DateTimeUnit.YEAR_OF_CENTURY);
74+
private static final Unit MONTH_OF_YEAR = Unit.from(Rounding.DateTimeUnit.MONTH_OF_YEAR);
75+
private static final Unit WEEK_OF_WEEKYEAR = Unit.from(Rounding.DateTimeUnit.WEEK_OF_WEEKYEAR);
76+
private static final List<Unit> DAY_OF_MONTH_OR_FINER = List.of(
77+
Unit.from(Rounding.DateTimeUnit.DAY_OF_MONTH),
78+
Unit.from(TimeValue.timeValueHours(12)),
79+
Unit.from(TimeValue.timeValueHours(3)),
80+
Unit.from(TimeValue.timeValueHours(1)),
81+
Unit.from(TimeValue.timeValueMinutes(30)),
82+
Unit.from(TimeValue.timeValueMinutes(10)),
83+
Unit.from(TimeValue.timeValueMinutes(5)),
84+
Unit.from(TimeValue.timeValueMinutes(1)),
85+
Unit.from(TimeValue.timeValueSeconds(30)),
86+
Unit.from(TimeValue.timeValueSeconds(10)),
87+
Unit.from(TimeValue.timeValueSeconds(5)),
88+
Unit.from(TimeValue.timeValueSeconds(1)),
89+
Unit.from(TimeValue.timeValueMillis(100)),
90+
Unit.from(TimeValue.timeValueMillis(50)),
91+
Unit.from(TimeValue.timeValueMillis(10)),
92+
Unit.from(TimeValue.timeValueMillis(1))
9593
);
9694

95+
private record Unit(Function<ZoneId, Rounding> fn) implements Function<ZoneId, Rounding> {
96+
97+
public static Unit from(Rounding.DateTimeUnit unit) {
98+
return new Unit(zoneId -> Rounding.builder(unit).timeZone(zoneId).build());
99+
}
100+
101+
public static Unit from(TimeValue interval) {
102+
return new Unit(zoneId -> Rounding.builder(interval).timeZone(zoneId).build());
103+
}
104+
105+
@Override
106+
public Rounding apply(ZoneId zoneId) {
107+
return fn.apply(zoneId);
108+
}
109+
}
110+
97111
private final Configuration configuration;
98112
private final Expression field;
99113
private final Expression buckets;
@@ -331,16 +345,36 @@ public Rounding.Prepared getDateRounding(FoldContext foldContext, Long min, Long
331345

332346
private record DateRoundingPicker(int buckets, long from, long to, ZoneId zoneId) {
333347
Rounding pickRounding() {
334-
Rounding prev = LARGEST_HUMAN_DATE_ROUNDING.apply(zoneId);
335-
for (Function<ZoneId, Rounding> roundingMaker : HUMAN_DATE_ROUNDINGS) {
336-
Rounding r = roundingMaker.apply(zoneId);
337-
if (roundingIsOk(r)) {
338-
prev = r;
348+
Rounding best = findLastOk(DAY_OF_MONTH_OR_FINER);
349+
if (best != null) {
350+
return best;
351+
}
352+
Rounding week = WEEK_OF_WEEKYEAR.apply(zoneId);
353+
if (roundingIsOk(week)) {
354+
return week;
355+
}
356+
Rounding month = MONTH_OF_YEAR.apply(zoneId);
357+
if (roundingIsOk(month)) {
358+
return month;
359+
}
360+
return YEAR_OF_CENTURY.apply(zoneId);
361+
}
362+
363+
private Rounding findLastOk(List<Unit> candidates) {
364+
int low = 0;
365+
int high = candidates.size() - 1;
366+
Rounding best = null;
367+
while (low <= high) {
368+
int mid = (low + high) >>> 1;
369+
Rounding candidate = candidates.get(mid).apply(zoneId);
370+
if (roundingIsOk(candidate)) {
371+
best = candidate;
372+
low = mid + 1;
339373
} else {
340-
return prev;
374+
high = mid - 1;
341375
}
342376
}
343-
return prev;
377+
return best;
344378
}
345379

346380
/**

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/grouping/BucketTests.java‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ public BucketTests(@Name("TestCase") Supplier<TestCaseSupplier.TestCase> testCas
5454
public static Iterable<Object[]> parameters() {
5555
List<TestCaseSupplier> suppliers = new ArrayList<>();
5656
dateCases(suppliers, "fixed date", () -> DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER.parseMillis("2023-02-17T09:00:00.00Z"));
57+
dateRoundingHandlesNonNestedCalendarUnits(suppliers);
5758
dateNanosCases(suppliers, "fixed date nanos", () -> DateUtils.toLong(Instant.parse("2023-02-17T09:00:00.00Z")));
5859
dateCasesWithSpan(
5960
suppliers,
@@ -144,6 +145,50 @@ private static void dateCases(List<TestCaseSupplier> suppliers, String name, Lon
144145
}
145146
}
146147

148+
private static void dateRoundingHandlesNonNestedCalendarUnits(List<TestCaseSupplier> suppliers) {
149+
suppliers.add(
150+
new TestCaseSupplier(
151+
"month boundary can still be weekly",
152+
List.of(DataType.DATETIME, DataType.INTEGER, DataType.DATETIME, DataType.DATETIME),
153+
() -> {
154+
long date = DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER.parseMillis("2024-02-01T12:00:00Z");
155+
List<TestCaseSupplier.TypedData> args = new ArrayList<>();
156+
args.add(new TestCaseSupplier.TypedData(date, DataType.DATETIME, "field"));
157+
args.add(new TestCaseSupplier.TypedData(1, DataType.INTEGER, "buckets").forceLiteral());
158+
args.add(dateBound("from", DataType.DATETIME, "2024-01-31T00:00:00Z"));
159+
args.add(dateBound("to", DataType.DATETIME, "2024-02-02T00:00:00Z"));
160+
return new TestCaseSupplier.TestCase(
161+
args,
162+
Matchers.containsString("rounding=Rounding[WEEK_OF_WEEKYEAR in Z][fixed to midnight]"),
163+
DataType.DATETIME,
164+
equalTo(Rounding.builder(Rounding.DateTimeUnit.WEEK_OF_WEEKYEAR).build().prepareForUnknown().round(date))
165+
).withConfiguration(TEST_SOURCE, configurationForTimezone(ZoneOffset.UTC));
166+
}
167+
)
168+
);
169+
170+
suppliers.add(
171+
new TestCaseSupplier(
172+
"week boundary can still be monthly",
173+
List.of(DataType.DATETIME, DataType.INTEGER, DataType.DATETIME, DataType.DATETIME),
174+
() -> {
175+
long date = DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER.parseMillis("2024-01-08T12:00:00Z");
176+
List<TestCaseSupplier.TypedData> args = new ArrayList<>();
177+
args.add(new TestCaseSupplier.TypedData(date, DataType.DATETIME, "field"));
178+
args.add(new TestCaseSupplier.TypedData(1, DataType.INTEGER, "buckets").forceLiteral());
179+
args.add(dateBound("from", DataType.DATETIME, "2024-01-07T00:00:00Z"));
180+
args.add(dateBound("to", DataType.DATETIME, "2024-01-09T00:00:00Z"));
181+
return new TestCaseSupplier.TestCase(
182+
args,
183+
Matchers.containsString("rounding=Rounding[MONTH_OF_YEAR in Z][fixed to midnight]"),
184+
DataType.DATETIME,
185+
equalTo(Rounding.builder(Rounding.DateTimeUnit.MONTH_OF_YEAR).build().prepareForUnknown().round(date))
186+
).withConfiguration(TEST_SOURCE, configurationForTimezone(ZoneOffset.UTC));
187+
}
188+
)
189+
);
190+
}
191+
147192
private static void dateTruncCases(List<TestCaseSupplier> suppliers) {
148193
makeTruncPeriodTestCases().stream().map(data -> {
149194
List<TestCaseSupplier> caseSuppliers = new ArrayList<>();

0 commit comments

Comments
 (0)