Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return an error when a rate aggregation cannot calculate bucket sizes #65429

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/reference/aggregations/metrics/rate-aggregation.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,11 @@ convert the bucket size into the rate. By default the interval of the `date_hist
`"rate": "quarter"`:: compatible with only with `month`, `quarter` and `year` calendar intervals
`"rate": "year"`:: compatible with only with `month`, `quarter` and `year` calendar intervals

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
and histogram interval have to be in the same group: [`second`, ` minute`, `hour`, `day`, `week`] or [`month`, `quarter`, `year`]. For
example, if the date histogram is `month` based, only rate intervals of `month`, `quarter` or `year` are supported. If the date histogram
is `day` based, only `second`, ` minute`, `hour`, `day`, and `week` rate intervals are supported.

==== Script

The `rate` aggregation also supports scripting. For example, if we need to adjust out prices before calculating rates, we could use
Expand Down
50 changes: 47 additions & 3 deletions server/src/main/java/org/elasticsearch/common/Rounding.java
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,10 @@ public interface Prepared {
* next rounded value in specified units if possible.
*/
double roundingSize(long utcMillis, DateTimeUnit timeUnit);
/**
* Returns the size of each rounding bucket in timeUnits.
*/
double roundingSize(DateTimeUnit timeUnit);
/**
* If this rounding mechanism precalculates rounding points then
* this array stores dates such that each date between each entry.
Expand Down Expand Up @@ -610,16 +614,41 @@ public String toString() {
private abstract class TimeUnitPreparedRounding extends PreparedRounding {
@Override
public double roundingSize(long utcMillis, DateTimeUnit timeUnit) {
if (timeUnit.isMillisBased == unit.isMillisBased) {
return (double) unit.ratio / timeUnit.ratio;
if (unit.isMillisBased) {
if (timeUnit.isMillisBased) {
return (double) unit.ratio / timeUnit.ratio;
} else {
throw new IllegalArgumentException("Cannot use month-based rate unit [" + timeUnit.shortName +
"] with non-month based calendar interval histogram [" + unit.shortName +
"] only week, day, hour, minute and second are supported for this histogram");
}
} else {
if (unit.isMillisBased == false) {
if (timeUnit.isMillisBased) {
return (double) (nextRoundingValue(utcMillis) - utcMillis) / timeUnit.ratio;
} else {
return (double) unit.ratio / timeUnit.ratio;
}
}
}

@Override
public double roundingSize(DateTimeUnit timeUnit) {
if (unit.isMillisBased) {
if (timeUnit.isMillisBased) {
return (double) unit.ratio / timeUnit.ratio;
} else {
throw new IllegalArgumentException("Cannot use month-based rate unit [" + timeUnit.shortName +
"] with non-month based calendar interval histogram [" + unit.shortName +
"] only week, day, hour, minute and second are supported for this histogram");
}
} else {
if (timeUnit.isMillisBased) {
throw new IllegalArgumentException("Cannot use non month-based rate unit [" + timeUnit.shortName +
"] with calendar interval histogram [" + unit.shortName +
"] only month, quarter and year are supported for this histogram");
} else {
return (double) unit.ratio / timeUnit.ratio;
}
}
}
}
Expand Down Expand Up @@ -996,6 +1025,11 @@ private long roundKey(long value, long interval) {
private abstract class TimeIntervalPreparedRounding extends PreparedRounding {
@Override
public double roundingSize(long utcMillis, DateTimeUnit timeUnit) {
return roundingSize(timeUnit);
}

@Override
public double roundingSize(DateTimeUnit timeUnit) {
if (timeUnit.isMillisBased) {
return (double) interval / timeUnit.ratio;
} else {
Expand Down Expand Up @@ -1266,6 +1300,11 @@ public double roundingSize(long utcMillis, DateTimeUnit timeUnit) {
return delegatePrepared.roundingSize(utcMillis, timeUnit);
}

@Override
public double roundingSize(DateTimeUnit timeUnit) {
return delegatePrepared.roundingSize(timeUnit);
}

@Override
public long[] fixedRoundingPoints() {
// TODO we can likely translate here
Expand Down Expand Up @@ -1354,6 +1393,11 @@ public double roundingSize(long utcMillis, DateTimeUnit timeUnit) {
return delegate.roundingSize(utcMillis, timeUnit);
}

@Override
public double roundingSize(DateTimeUnit timeUnit) {
return delegate.roundingSize(timeUnit);
}

@Override
public long[] fixedRoundingPoints() {
return Arrays.copyOf(values, max);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,15 @@ public double bucketSize(long bucket, Rounding.DateTimeUnit unitSize) {
}
}

@Override
public double bucketSize(Rounding.DateTimeUnit unitSize) {
if (unitSize != null) {
return preparedRounding.roundingSize(unitSize);
} else {
return 1.0;
}
}

static class FromDateRange extends AdaptingAggregator implements SizedBucketAggregator {
private final DocValueFormat format;
private final Rounding rounding;
Expand Down Expand Up @@ -431,5 +440,14 @@ public double bucketSize(long bucket, DateTimeUnit unitSize) {
return 1.0;
}
}

@Override
public double bucketSize(DateTimeUnit unitSize) {
if (unitSize != null) {
return preparedRounding.roundingSize(unitSize);
} else {
return 1.0;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,17 @@
import org.elasticsearch.common.Rounding;

/**
* An aggregator capable of reporting bucket sizes in milliseconds. Used by RateAggregator for calendar-based buckets.
* An aggregator capable of reporting bucket sizes in requested units. Used by RateAggregator for calendar-based buckets.
*/
public interface SizedBucketAggregator {
/**
* Reports size of the particular bucket in requested units.
*/
double bucketSize(long bucket, Rounding.DateTimeUnit unit);

/**
* Reports size of all buckets in requested units. Throws an exception if it is not possible to calculate without knowing
* the concrete bucket.
*/
double bucketSize(Rounding.DateTimeUnit unit);
}
35 changes: 33 additions & 2 deletions server/src/test/java/org/elasticsearch/common/RoundingTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -981,24 +981,48 @@ public void testMillisecondsBasedUnitCalendarRoundingSize() {
Rounding.Prepared prepared = unitRounding.prepare(time("2010-01-01T00:00:00.000Z"), time("2020-01-01T00:00:00.000Z"));
assertThat(prepared.roundingSize(time("2015-01-01T00:00:00.000Z"), Rounding.DateTimeUnit.SECOND_OF_MINUTE),
closeTo(3600.0, 0.000001));
assertThat(prepared.roundingSize(time("2015-01-01T00:00:00.000Z"), Rounding.DateTimeUnit.MINUTES_OF_HOUR), closeTo(60.0, 0.000001));
assertThat(prepared.roundingSize(time("2015-01-01T00:00:00.000Z"), Rounding.DateTimeUnit.HOUR_OF_DAY), closeTo(1.0, 0.000001));
assertThat(prepared.roundingSize(Rounding.DateTimeUnit.SECOND_OF_MINUTE),
closeTo(3600.0, 0.000001));
assertThat(prepared.roundingSize(time("2015-01-01T00:00:00.000Z"), Rounding.DateTimeUnit.MINUTES_OF_HOUR),
closeTo(60.0, 0.000001));
assertThat(prepared.roundingSize(Rounding.DateTimeUnit.MINUTES_OF_HOUR),
closeTo(60.0, 0.000001));
assertThat(prepared.roundingSize(time("2015-01-01T00:00:00.000Z"), Rounding.DateTimeUnit.HOUR_OF_DAY),
closeTo(1.0, 0.000001));
assertThat(prepared.roundingSize(Rounding.DateTimeUnit.HOUR_OF_DAY),
closeTo(1.0, 0.000001));
assertThat(prepared.roundingSize(time("2015-01-01T00:00:00.000Z"), Rounding.DateTimeUnit.DAY_OF_MONTH),
closeTo(1 / 24.0, 0.000001));
assertThat(prepared.roundingSize(Rounding.DateTimeUnit.DAY_OF_MONTH),
closeTo(1 / 24.0, 0.000001));
assertThat(prepared.roundingSize(time("2015-01-01T00:00:00.000Z"), Rounding.DateTimeUnit.WEEK_OF_WEEKYEAR),
closeTo(1 / 168.0, 0.000001));
assertThat(prepared.roundingSize(Rounding.DateTimeUnit.WEEK_OF_WEEKYEAR),
closeTo(1 / 168.0, 0.000001));
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class,
() -> prepared.roundingSize(time("2015-01-01T00:00:00.000Z"), Rounding.DateTimeUnit.MONTH_OF_YEAR));
assertThat(ex.getMessage(), equalTo("Cannot use month-based rate unit [month] with non-month based calendar interval " +
"histogram [hour] only week, day, hour, minute and second are supported for this histogram"));
ex = expectThrows(IllegalArgumentException.class,
() -> prepared.roundingSize(Rounding.DateTimeUnit.MONTH_OF_YEAR));
assertThat(ex.getMessage(), equalTo("Cannot use month-based rate unit [month] with non-month based calendar interval " +
"histogram [hour] only week, day, hour, minute and second are supported for this histogram"));
ex = expectThrows(IllegalArgumentException.class,
() -> prepared.roundingSize(time("2015-01-01T00:00:00.000Z"), Rounding.DateTimeUnit.QUARTER_OF_YEAR));
assertThat(ex.getMessage(), equalTo("Cannot use month-based rate unit [quarter] with non-month based calendar interval " +
"histogram [hour] only week, day, hour, minute and second are supported for this histogram"));
ex = expectThrows(IllegalArgumentException.class,
() -> prepared.roundingSize(Rounding.DateTimeUnit.QUARTER_OF_YEAR));
assertThat(ex.getMessage(), equalTo("Cannot use month-based rate unit [quarter] with non-month based calendar interval " +
"histogram [hour] only week, day, hour, minute and second are supported for this histogram"));
ex = expectThrows(IllegalArgumentException.class,
() -> prepared.roundingSize(time("2015-01-01T00:00:00.000Z"), Rounding.DateTimeUnit.YEAR_OF_CENTURY));
assertThat(ex.getMessage(), equalTo("Cannot use month-based rate unit [year] with non-month based calendar interval " +
"histogram [hour] only week, day, hour, minute and second are supported for this histogram"));
ex = expectThrows(IllegalArgumentException.class,
() -> prepared.roundingSize(Rounding.DateTimeUnit.YEAR_OF_CENTURY));
assertThat(ex.getMessage(), equalTo("Cannot use month-based rate unit [year] with non-month based calendar interval " +
"histogram [hour] only week, day, hour, minute and second are supported for this histogram"));
}

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

public void testFixedRoundingPoints() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,23 @@ public double metric(long owningBucketOrd) {
if (sizedBucketAggregator == null || valuesSource == null || owningBucketOrd >= sums.size()) {
return 0.0;
}
return sums.get(owningBucketOrd) / sizedBucketAggregator.bucketSize(owningBucketOrd, rateUnit);
return sums.get(owningBucketOrd) / divisor(owningBucketOrd);
}

@Override
public InternalAggregation buildAggregation(long bucket) {
if (valuesSource == null || bucket >= sums.size()) {
return buildEmptyAggregation();
}
return new InternalRate(name, sums.get(bucket), sizedBucketAggregator.bucketSize(bucket, rateUnit), format, metadata());
return new InternalRate(name, sums.get(bucket), divisor(bucket), format, metadata());
}

private double divisor(long owningBucketOrd) {
if (sizedBucketAggregator == parent) {
return sizedBucketAggregator.bucketSize(owningBucketOrd, rateUnit);
} else {
return sizedBucketAggregator.bucketSize(rateUnit);
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,77 @@ public void testKeywordSandwich() throws IOException {
}, dateType, numType, keywordType);
}

public void testUnsupportedKeywordSandwich() throws IOException {
String rate;
String histogram;
boolean millisecondBasedRate = randomBoolean();
if (millisecondBasedRate) {
rate = randomFrom("second", "minute", "day", "week");
histogram = randomFrom("month", "quarter", "year");
} else {
rate = randomFrom("month", "quarter", "year");
histogram = randomFrom("second", "minute", "day", "week");
}

MappedFieldType numType = new NumberFieldMapper.NumberFieldType("val", NumberFieldMapper.NumberType.INTEGER);
MappedFieldType dateType = dateFieldType(DATE_FIELD);
MappedFieldType keywordType = new KeywordFieldMapper.KeywordFieldType("term");
RateAggregationBuilder rateAggregationBuilder = new RateAggregationBuilder("my_rate").rateUnit(rate).field("val");
if (randomBoolean()) {
rateAggregationBuilder.rateMode("sum");
}
TermsAggregationBuilder termsAggregationBuilder = new TermsAggregationBuilder("my_term").field("term")
.subAggregation(rateAggregationBuilder);
DateHistogramAggregationBuilder dateHistogramAggregationBuilder = new DateHistogramAggregationBuilder("my_date").field(DATE_FIELD)
.calendarInterval(new DateHistogramInterval(histogram))
.subAggregation(termsAggregationBuilder);

IllegalArgumentException ex = expectThrows(
IllegalArgumentException.class, () -> testCase(dateHistogramAggregationBuilder, new MatchAllDocsQuery(), iw -> {
iw.addDocument(
doc(
"2010-03-11T01:07:45",
new NumericDocValuesField("val", 1),
new IntPoint("val", 1),
new SortedSetDocValuesField("term", new BytesRef("a"))
)
);
iw.addDocument(
doc(
"2010-03-12T01:07:45",
new NumericDocValuesField("val", 2),
new IntPoint("val", 2),
new SortedSetDocValuesField("term", new BytesRef("a"))
)
);
iw.addDocument(
doc(
"2010-04-01T03:43:34",
new NumericDocValuesField("val", 3),
new IntPoint("val", 3),
new SortedSetDocValuesField("term", new BytesRef("a"))
)
);
iw.addDocument(
doc(
"2010-04-27T03:43:34",
new NumericDocValuesField("val", 4),
new IntPoint("val", 4),
new SortedSetDocValuesField("term", new BytesRef("b"))
)
);
}, (Consumer<InternalDateHistogram>) dh -> {
fail("Shouldn't be here");
}, dateType, numType, keywordType));
if (millisecondBasedRate) {
assertEquals("Cannot use non month-based rate unit [" + rate + "] with calendar interval histogram [" +
histogram + "] only month, quarter and year are supported for this histogram", ex.getMessage());
} else {
assertEquals("Cannot use month-based rate unit [" + rate + "] with non-month based calendar interval histogram [" +
histogram + "] only week, day, hour, minute and second are supported for this histogram", ex.getMessage());
}
}

public void testKeywordSandwichWithSorting() throws IOException {
MappedFieldType numType = new NumberFieldMapper.NumberFieldType("val", NumberFieldMapper.NumberType.INTEGER);
MappedFieldType dateType = dateFieldType(DATE_FIELD);
Expand Down