Skip to content

Commit de3ee05

Browse files
imotovimotov
authored
Return an error when a rate aggregation cannot calculate bucket sizes (#65429) (#65502)
In some cases when the rate aggregation is not a child of a date histogram aggregation, it is not possible to determine the actual size of the date histogram bucket. In this case the rate aggregation now throws an exception. Closes #63703
1 parent c7738e0 commit de3ee05

File tree

7 files changed

+194
-8
lines changed

7 files changed

+194
-8
lines changed

‎docs/reference/aggregations/metrics/rate-aggregation.asciidoc

+5
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,11 @@ convert the bucket size into the rate. By default the interval of the `date_hist
261261
`"rate": "quarter"`:: compatible with only with `month`, `quarter` and `year` calendar intervals
262262
`"rate": "year"`:: compatible with only with `month`, `quarter` and `year` calendar intervals
263263

264+
There is also an additional limitations if the date histogram is not a direct parent of the rate histogram. In this case both rate interval
265+
and histogram interval have to be in the same group: [`second`, ` minute`, `hour`, `day`, `week`] or [`month`, `quarter`, `year`]. For
266+
example, if the date histogram is `month` based, only rate intervals of `month`, `quarter` or `year` are supported. If the date histogram
267+
is `day` based, only `second`, ` minute`, `hour`, `day`, and `week` rate intervals are supported.
268+
264269
==== Script
265270

266271
The `rate` aggregation also supports scripting. For example, if we need to adjust out prices before calculating rates, we could use

‎server/src/main/java/org/elasticsearch/common/Rounding.java

+47-3
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,10 @@ public interface Prepared {
292292
* next rounded value in specified units if possible.
293293
*/
294294
double roundingSize(long utcMillis, DateTimeUnit timeUnit);
295+
/**
296+
* Returns the size of each rounding bucket in timeUnits.
297+
*/
298+
double roundingSize(DateTimeUnit timeUnit);
295299
/**
296300
* If this rounding mechanism precalculates rounding points then
297301
* this array stores dates such that each date between each entry.
@@ -616,16 +620,41 @@ public String toString() {
616620
private abstract class TimeUnitPreparedRounding extends PreparedRounding {
617621
@Override
618622
public double roundingSize(long utcMillis, DateTimeUnit timeUnit) {
619-
if (timeUnit.isMillisBased == unit.isMillisBased) {
620-
return (double) unit.ratio / timeUnit.ratio;
623+
if (unit.isMillisBased) {
624+
if (timeUnit.isMillisBased) {
625+
return (double) unit.ratio / timeUnit.ratio;
626+
} else {
627+
throw new IllegalArgumentException("Cannot use month-based rate unit [" + timeUnit.shortName +
628+
"] with non-month based calendar interval histogram [" + unit.shortName +
629+
"] only week, day, hour, minute and second are supported for this histogram");
630+
}
621631
} else {
622-
if (unit.isMillisBased == false) {
632+
if (timeUnit.isMillisBased) {
623633
return (double) (nextRoundingValue(utcMillis) - utcMillis) / timeUnit.ratio;
634+
} else {
635+
return (double) unit.ratio / timeUnit.ratio;
636+
}
637+
}
638+
}
639+
640+
@Override
641+
public double roundingSize(DateTimeUnit timeUnit) {
642+
if (unit.isMillisBased) {
643+
if (timeUnit.isMillisBased) {
644+
return (double) unit.ratio / timeUnit.ratio;
624645
} else {
625646
throw new IllegalArgumentException("Cannot use month-based rate unit [" + timeUnit.shortName +
626647
"] with non-month based calendar interval histogram [" + unit.shortName +
627648
"] only week, day, hour, minute and second are supported for this histogram");
628649
}
650+
} else {
651+
if (timeUnit.isMillisBased) {
652+
throw new IllegalArgumentException("Cannot use non month-based rate unit [" + timeUnit.shortName +
653+
"] with calendar interval histogram [" + unit.shortName +
654+
"] only month, quarter and year are supported for this histogram");
655+
} else {
656+
return (double) unit.ratio / timeUnit.ratio;
657+
}
629658
}
630659
}
631660
}
@@ -1007,6 +1036,11 @@ private long roundKey(long value, long interval) {
10071036
private abstract class TimeIntervalPreparedRounding extends PreparedRounding {
10081037
@Override
10091038
public double roundingSize(long utcMillis, DateTimeUnit timeUnit) {
1039+
return roundingSize(timeUnit);
1040+
}
1041+
1042+
@Override
1043+
public double roundingSize(DateTimeUnit timeUnit) {
10101044
if (timeUnit.isMillisBased) {
10111045
return (double) interval / timeUnit.ratio;
10121046
} else {
@@ -1280,6 +1314,11 @@ public double roundingSize(long utcMillis, DateTimeUnit timeUnit) {
12801314
return delegatePrepared.roundingSize(utcMillis, timeUnit);
12811315
}
12821316

1317+
@Override
1318+
public double roundingSize(DateTimeUnit timeUnit) {
1319+
return delegatePrepared.roundingSize(timeUnit);
1320+
}
1321+
12831322
@Override
12841323
public long[] fixedRoundingPoints() {
12851324
// TODO we can likely translate here
@@ -1368,6 +1407,11 @@ public double roundingSize(long utcMillis, DateTimeUnit timeUnit) {
13681407
return delegate.roundingSize(utcMillis, timeUnit);
13691408
}
13701409

1410+
@Override
1411+
public double roundingSize(DateTimeUnit timeUnit) {
1412+
return delegate.roundingSize(timeUnit);
1413+
}
1414+
13711415
@Override
13721416
public long[] fixedRoundingPoints() {
13731417
return Arrays.copyOf(values, max);

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/DateHistogramAggregator.java

+18
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,15 @@ public double bucketSize(long bucket, Rounding.DateTimeUnit unitSize) {
344344
}
345345
}
346346

347+
@Override
348+
public double bucketSize(Rounding.DateTimeUnit unitSize) {
349+
if (unitSize != null) {
350+
return preparedRounding.roundingSize(unitSize);
351+
} else {
352+
return 1.0;
353+
}
354+
}
355+
347356
static class FromDateRange extends AdaptingAggregator implements SizedBucketAggregator {
348357
private final DocValueFormat format;
349358
private final Rounding rounding;
@@ -431,5 +440,14 @@ public double bucketSize(long bucket, DateTimeUnit unitSize) {
431440
return 1.0;
432441
}
433442
}
443+
444+
@Override
445+
public double bucketSize(DateTimeUnit unitSize) {
446+
if (unitSize != null) {
447+
return preparedRounding.roundingSize(unitSize);
448+
} else {
449+
return 1.0;
450+
}
451+
}
434452
}
435453
}

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/histogram/SizedBucketAggregator.java

+10-1
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,17 @@
2222
import org.elasticsearch.common.Rounding;
2323

2424
/**
25-
* An aggregator capable of reporting bucket sizes in milliseconds. Used by RateAggregator for calendar-based buckets.
25+
* An aggregator capable of reporting bucket sizes in requested units. Used by RateAggregator for calendar-based buckets.
2626
*/
2727
public interface SizedBucketAggregator {
28+
/**
29+
* Reports size of the particular bucket in requested units.
30+
*/
2831
double bucketSize(long bucket, Rounding.DateTimeUnit unit);
32+
33+
/**
34+
* Reports size of all buckets in requested units. Throws an exception if it is not possible to calculate without knowing
35+
* the concrete bucket.
36+
*/
37+
double bucketSize(Rounding.DateTimeUnit unit);
2938
}

‎server/src/test/java/org/elasticsearch/common/RoundingTests.java

+33-2
Original file line numberDiff line numberDiff line change
@@ -981,24 +981,48 @@ public void testMillisecondsBasedUnitCalendarRoundingSize() {
981981
Rounding.Prepared prepared = unitRounding.prepare(time("2010-01-01T00:00:00.000Z"), time("2020-01-01T00:00:00.000Z"));
982982
assertThat(prepared.roundingSize(time("2015-01-01T00:00:00.000Z"), Rounding.DateTimeUnit.SECOND_OF_MINUTE),
983983
closeTo(3600.0, 0.000001));
984-
assertThat(prepared.roundingSize(time("2015-01-01T00:00:00.000Z"), Rounding.DateTimeUnit.MINUTES_OF_HOUR), closeTo(60.0, 0.000001));
985-
assertThat(prepared.roundingSize(time("2015-01-01T00:00:00.000Z"), Rounding.DateTimeUnit.HOUR_OF_DAY), closeTo(1.0, 0.000001));
984+
assertThat(prepared.roundingSize(Rounding.DateTimeUnit.SECOND_OF_MINUTE),
985+
closeTo(3600.0, 0.000001));
986+
assertThat(prepared.roundingSize(time("2015-01-01T00:00:00.000Z"), Rounding.DateTimeUnit.MINUTES_OF_HOUR),
987+
closeTo(60.0, 0.000001));
988+
assertThat(prepared.roundingSize(Rounding.DateTimeUnit.MINUTES_OF_HOUR),
989+
closeTo(60.0, 0.000001));
990+
assertThat(prepared.roundingSize(time("2015-01-01T00:00:00.000Z"), Rounding.DateTimeUnit.HOUR_OF_DAY),
991+
closeTo(1.0, 0.000001));
992+
assertThat(prepared.roundingSize(Rounding.DateTimeUnit.HOUR_OF_DAY),
993+
closeTo(1.0, 0.000001));
986994
assertThat(prepared.roundingSize(time("2015-01-01T00:00:00.000Z"), Rounding.DateTimeUnit.DAY_OF_MONTH),
987995
closeTo(1 / 24.0, 0.000001));
996+
assertThat(prepared.roundingSize(Rounding.DateTimeUnit.DAY_OF_MONTH),
997+
closeTo(1 / 24.0, 0.000001));
988998
assertThat(prepared.roundingSize(time("2015-01-01T00:00:00.000Z"), Rounding.DateTimeUnit.WEEK_OF_WEEKYEAR),
989999
closeTo(1 / 168.0, 0.000001));
1000+
assertThat(prepared.roundingSize(Rounding.DateTimeUnit.WEEK_OF_WEEKYEAR),
1001+
closeTo(1 / 168.0, 0.000001));
9901002
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class,
9911003
() -> prepared.roundingSize(time("2015-01-01T00:00:00.000Z"), Rounding.DateTimeUnit.MONTH_OF_YEAR));
9921004
assertThat(ex.getMessage(), equalTo("Cannot use month-based rate unit [month] with non-month based calendar interval " +
9931005
"histogram [hour] only week, day, hour, minute and second are supported for this histogram"));
1006+
ex = expectThrows(IllegalArgumentException.class,
1007+
() -> prepared.roundingSize(Rounding.DateTimeUnit.MONTH_OF_YEAR));
1008+
assertThat(ex.getMessage(), equalTo("Cannot use month-based rate unit [month] with non-month based calendar interval " +
1009+
"histogram [hour] only week, day, hour, minute and second are supported for this histogram"));
9941010
ex = expectThrows(IllegalArgumentException.class,
9951011
() -> prepared.roundingSize(time("2015-01-01T00:00:00.000Z"), Rounding.DateTimeUnit.QUARTER_OF_YEAR));
9961012
assertThat(ex.getMessage(), equalTo("Cannot use month-based rate unit [quarter] with non-month based calendar interval " +
9971013
"histogram [hour] only week, day, hour, minute and second are supported for this histogram"));
1014+
ex = expectThrows(IllegalArgumentException.class,
1015+
() -> prepared.roundingSize(Rounding.DateTimeUnit.QUARTER_OF_YEAR));
1016+
assertThat(ex.getMessage(), equalTo("Cannot use month-based rate unit [quarter] with non-month based calendar interval " +
1017+
"histogram [hour] only week, day, hour, minute and second are supported for this histogram"));
9981018
ex = expectThrows(IllegalArgumentException.class,
9991019
() -> prepared.roundingSize(time("2015-01-01T00:00:00.000Z"), Rounding.DateTimeUnit.YEAR_OF_CENTURY));
10001020
assertThat(ex.getMessage(), equalTo("Cannot use month-based rate unit [year] with non-month based calendar interval " +
10011021
"histogram [hour] only week, day, hour, minute and second are supported for this histogram"));
1022+
ex = expectThrows(IllegalArgumentException.class,
1023+
() -> prepared.roundingSize(Rounding.DateTimeUnit.YEAR_OF_CENTURY));
1024+
assertThat(ex.getMessage(), equalTo("Cannot use month-based rate unit [year] with non-month based calendar interval " +
1025+
"histogram [hour] only week, day, hour, minute and second are supported for this histogram"));
10021026
}
10031027

10041028
public void testNonMillisecondsBasedUnitCalendarRoundingSize() {
@@ -1009,6 +1033,9 @@ public void testNonMillisecondsBasedUnitCalendarRoundingSize() {
10091033
assertThat(prepared.roundingSize(firstQuarter, Rounding.DateTimeUnit.MONTH_OF_YEAR), closeTo(3.0, 0.000001));
10101034
assertThat(prepared.roundingSize(firstQuarter, Rounding.DateTimeUnit.QUARTER_OF_YEAR), closeTo(1.0, 0.000001));
10111035
assertThat(prepared.roundingSize(firstQuarter, Rounding.DateTimeUnit.YEAR_OF_CENTURY), closeTo(0.25, 0.000001));
1036+
assertThat(prepared.roundingSize(Rounding.DateTimeUnit.MONTH_OF_YEAR), closeTo(3.0, 0.000001));
1037+
assertThat(prepared.roundingSize(Rounding.DateTimeUnit.QUARTER_OF_YEAR), closeTo(1.0, 0.000001));
1038+
assertThat(prepared.roundingSize(Rounding.DateTimeUnit.YEAR_OF_CENTURY), closeTo(0.25, 0.000001));
10121039
// Real interval based
10131040
assertThat(prepared.roundingSize(firstQuarter, Rounding.DateTimeUnit.SECOND_OF_MINUTE), closeTo(7776000.0, 0.000001));
10141041
assertThat(prepared.roundingSize(firstQuarter, Rounding.DateTimeUnit.MINUTES_OF_HOUR), closeTo(129600.0, 0.000001));
@@ -1017,6 +1044,10 @@ public void testNonMillisecondsBasedUnitCalendarRoundingSize() {
10171044
long thirdQuarter = prepared.round(time("2015-07-01T00:00:00.000Z"));
10181045
assertThat(prepared.roundingSize(thirdQuarter, Rounding.DateTimeUnit.DAY_OF_MONTH), closeTo(92.0, 0.000001));
10191046
assertThat(prepared.roundingSize(thirdQuarter, Rounding.DateTimeUnit.HOUR_OF_DAY), closeTo(2208.0, 0.000001));
1047+
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class,
1048+
() -> prepared.roundingSize(Rounding.DateTimeUnit.SECOND_OF_MINUTE));
1049+
assertThat(ex.getMessage(), equalTo("Cannot use non month-based rate unit [second] with calendar interval histogram " +
1050+
"[quarter] only month, quarter and year are supported for this histogram"));
10201051
}
10211052

10221053
public void testFixedRoundingPoints() {

‎x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/rate/AbstractRateAggregator.java

+10-2
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,23 @@ public double metric(long owningBucketOrd) {
8080
if (sizedBucketAggregator == null || valuesSource == null || owningBucketOrd >= sums.size()) {
8181
return 0.0;
8282
}
83-
return sums.get(owningBucketOrd) / sizedBucketAggregator.bucketSize(owningBucketOrd, rateUnit);
83+
return sums.get(owningBucketOrd) / divisor(owningBucketOrd);
8484
}
8585

8686
@Override
8787
public InternalAggregation buildAggregation(long bucket) {
8888
if (valuesSource == null || bucket >= sums.size()) {
8989
return buildEmptyAggregation();
9090
}
91-
return new InternalRate(name, sums.get(bucket), sizedBucketAggregator.bucketSize(bucket, rateUnit), format, metadata());
91+
return new InternalRate(name, sums.get(bucket), divisor(bucket), format, metadata());
92+
}
93+
94+
private double divisor(long owningBucketOrd) {
95+
if (sizedBucketAggregator == parent) {
96+
return sizedBucketAggregator.bucketSize(owningBucketOrd, rateUnit);
97+
} else {
98+
return sizedBucketAggregator.bucketSize(rateUnit);
99+
}
92100
}
93101

94102
@Override

‎x-pack/plugin/analytics/src/test/java/org/elasticsearch/xpack/analytics/rate/RateAggregatorTests.java

+71
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,77 @@ public void testKeywordSandwich() throws IOException {
377377
}, dateType, numType, keywordType);
378378
}
379379

380+
public void testUnsupportedKeywordSandwich() throws IOException {
381+
String rate;
382+
String histogram;
383+
boolean millisecondBasedRate = randomBoolean();
384+
if (millisecondBasedRate) {
385+
rate = randomFrom("second", "minute", "day", "week");
386+
histogram = randomFrom("month", "quarter", "year");
387+
} else {
388+
rate = randomFrom("month", "quarter", "year");
389+
histogram = randomFrom("second", "minute", "day", "week");
390+
}
391+
392+
MappedFieldType numType = new NumberFieldMapper.NumberFieldType("val", NumberFieldMapper.NumberType.INTEGER);
393+
MappedFieldType dateType = dateFieldType(DATE_FIELD);
394+
MappedFieldType keywordType = new KeywordFieldMapper.KeywordFieldType("term");
395+
RateAggregationBuilder rateAggregationBuilder = new RateAggregationBuilder("my_rate").rateUnit(rate).field("val");
396+
if (randomBoolean()) {
397+
rateAggregationBuilder.rateMode("sum");
398+
}
399+
TermsAggregationBuilder termsAggregationBuilder = new TermsAggregationBuilder("my_term").field("term")
400+
.subAggregation(rateAggregationBuilder);
401+
DateHistogramAggregationBuilder dateHistogramAggregationBuilder = new DateHistogramAggregationBuilder("my_date").field(DATE_FIELD)
402+
.calendarInterval(new DateHistogramInterval(histogram))
403+
.subAggregation(termsAggregationBuilder);
404+
405+
IllegalArgumentException ex = expectThrows(
406+
IllegalArgumentException.class, () -> testCase(dateHistogramAggregationBuilder, new MatchAllDocsQuery(), iw -> {
407+
iw.addDocument(
408+
doc(
409+
"2010-03-11T01:07:45",
410+
new NumericDocValuesField("val", 1),
411+
new IntPoint("val", 1),
412+
new SortedSetDocValuesField("term", new BytesRef("a"))
413+
)
414+
);
415+
iw.addDocument(
416+
doc(
417+
"2010-03-12T01:07:45",
418+
new NumericDocValuesField("val", 2),
419+
new IntPoint("val", 2),
420+
new SortedSetDocValuesField("term", new BytesRef("a"))
421+
)
422+
);
423+
iw.addDocument(
424+
doc(
425+
"2010-04-01T03:43:34",
426+
new NumericDocValuesField("val", 3),
427+
new IntPoint("val", 3),
428+
new SortedSetDocValuesField("term", new BytesRef("a"))
429+
)
430+
);
431+
iw.addDocument(
432+
doc(
433+
"2010-04-27T03:43:34",
434+
new NumericDocValuesField("val", 4),
435+
new IntPoint("val", 4),
436+
new SortedSetDocValuesField("term", new BytesRef("b"))
437+
)
438+
);
439+
}, (Consumer<InternalDateHistogram>) dh -> {
440+
fail("Shouldn't be here");
441+
}, dateType, numType, keywordType));
442+
if (millisecondBasedRate) {
443+
assertEquals("Cannot use non month-based rate unit [" + rate + "] with calendar interval histogram [" +
444+
histogram + "] only month, quarter and year are supported for this histogram", ex.getMessage());
445+
} else {
446+
assertEquals("Cannot use month-based rate unit [" + rate + "] with non-month based calendar interval histogram [" +
447+
histogram + "] only week, day, hour, minute and second are supported for this histogram", ex.getMessage());
448+
}
449+
}
450+
380451
public void testKeywordSandwichWithSorting() throws IOException {
381452
MappedFieldType numType = new NumberFieldMapper.NumberFieldType("val", NumberFieldMapper.NumberType.INTEGER);
382453
MappedFieldType dateType = dateFieldType(DATE_FIELD);

0 commit comments

Comments
 (0)