Skip to content

Commit

Permalink
Optimize date_historam's hard_bounds (backport of #66051) (#66061)
Browse files Browse the repository at this point in the history
This allows `date_histogram`s with `hard_bounds` and `extended_bounds`
to use the "as range" style optimizations introducedin #63643. There
isn't any work to do for `exended_bounds` besides add a test. For
`hard_bounds` we have to be careful when constructing the ranges that to
filter.
  • Loading branch information
nik9000 authored Dec 8, 2020
1 parent cb1f603 commit bcb8ca0
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,6 @@ private static FromDateRange adaptIntoRangeOrNull(
CardinalityUpperBound cardinality,
Map<String, Object> metadata
) throws IOException {
if (hardBounds != null || extendedBounds != null) {
return null;
}
long[] fixedRoundingPoints = preparedRounding.fixedRoundingPoints();
if (fixedRoundingPoints == null) {
return null;
Expand All @@ -169,11 +166,7 @@ private static FromDateRange adaptIntoRangeOrNull(
if (rangeSupplier == null) {
return null;
}
RangeAggregator.Range[] ranges = new RangeAggregator.Range[fixedRoundingPoints.length];
for (int i = 0; i < fixedRoundingPoints.length - 1; i++) {
ranges[i] = new RangeAggregator.Range(null, (double) fixedRoundingPoints[i], (double) fixedRoundingPoints[i + 1]);
}
ranges[ranges.length - 1] = new RangeAggregator.Range(null, (double) fixedRoundingPoints[fixedRoundingPoints.length - 1], null);
RangeAggregator.Range[] ranges = ranges(hardBounds, fixedRoundingPoints);
return new DateHistogramAggregator.FromDateRange(
parent,
factories,
Expand All @@ -200,6 +193,27 @@ private static FromDateRange adaptIntoRangeOrNull(
);
}

private static RangeAggregator.Range[] ranges(LongBounds hardBounds, long[] fixedRoundingPoints) {
if (hardBounds == null) {
RangeAggregator.Range[] ranges = new RangeAggregator.Range[fixedRoundingPoints.length];
for (int i = 0; i < fixedRoundingPoints.length - 1; i++) {
ranges[i] = new RangeAggregator.Range(null, (double) fixedRoundingPoints[i], (double) fixedRoundingPoints[i + 1]);
}
ranges[ranges.length - 1] = new RangeAggregator.Range(null, (double) fixedRoundingPoints[fixedRoundingPoints.length - 1], null);
return ranges;
}
List<RangeAggregator.Range> ranges = new ArrayList<>(fixedRoundingPoints.length);
for (int i = 0; i < fixedRoundingPoints.length - 1; i++) {
if (hardBounds.contain(fixedRoundingPoints[i])) {
ranges.add(new RangeAggregator.Range(null, (double) fixedRoundingPoints[i], (double) fixedRoundingPoints[i + 1]));
}
}
if (hardBounds.contain(fixedRoundingPoints[fixedRoundingPoints.length - 1])) {
ranges.add(new RangeAggregator.Range(null, (double) fixedRoundingPoints[fixedRoundingPoints.length - 1], null));
}
return ranges.toArray(new RangeAggregator.Range[0]);
}

private final ValuesSource.Numeric valuesSource;
private final DocValueFormat formatter;
private final Rounding rounding;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ private void addEmptyBuckets(List<Bucket> list, ReduceContext reduceContext) {
LongBounds bounds = emptyBucketInfo.bounds;
ListIterator<Bucket> iter = list.listIterator();

// first adding all the empty buckets *before* the actual data (based on th extended_bounds.min the user requested)
// first adding all the empty buckets *before* the actual data (based on the extended_bounds.min the user requested)
InternalAggregations reducedEmptySubAggs = InternalAggregations.reduce(Collections.singletonList(emptyBucketInfo.subAggregations),
reduceContext);
if (bounds != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@
import org.elasticsearch.search.aggregations.AggregationBuilder;
import org.elasticsearch.search.aggregations.Aggregator;
import org.elasticsearch.search.aggregations.BucketOrder;
import org.elasticsearch.search.aggregations.InternalAggregation.ReduceContext;
import org.elasticsearch.search.aggregations.bucket.terms.StringTerms;
import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder;
import org.elasticsearch.search.aggregations.pipeline.PipelineAggregator.PipelineTree;
import org.elasticsearch.search.aggregations.support.AggregationContext;
import org.elasticsearch.search.aggregations.support.AggregationInspectionHelper;
import org.hamcrest.Matcher;
Expand Down Expand Up @@ -1200,9 +1202,44 @@ public void testMissingValueDoesNotUseFromRange() throws IOException {
);
}

public void testExtendedBoundsUsesFromRange() throws IOException {
aggregationImplementationChoiceTestCase(
aggregableDateFieldType(false, true, DateFormatter.forPattern("yyyy")),
org.elasticsearch.common.collect.List.of("2017", "2018"),
org.elasticsearch.common.collect.List.of("2016", "2017", "2018", "2019"),
new DateHistogramAggregationBuilder("test").field(AGGREGABLE_DATE)
.calendarInterval(DateHistogramInterval.YEAR)
.extendedBounds(new LongBounds("2016", "2019"))
.minDocCount(0),
true
);
}

public void testHardBoundsUsesFromRange() throws IOException {
aggregationImplementationChoiceTestCase(
aggregableDateFieldType(false, true, DateFormatter.forPattern("yyyy")),
org.elasticsearch.common.collect.List.of("2016", "2017", "2018", "2019"),
org.elasticsearch.common.collect.List.of("2017", "2018"),
new DateHistogramAggregationBuilder("test").field(AGGREGABLE_DATE)
.calendarInterval(DateHistogramInterval.YEAR)
.hardBounds(new LongBounds("2017", "2019")),
true
);
}

private void aggregationImplementationChoiceTestCase(
DateFieldMapper.DateFieldType ft,
List<String> data,
DateHistogramAggregationBuilder builder,
boolean usesFromRange
) throws IOException {
aggregationImplementationChoiceTestCase(ft, data, data, builder, usesFromRange);
}

private void aggregationImplementationChoiceTestCase(
DateFieldMapper.DateFieldType ft,
List<String> data,
List<String> resultingBucketKeys,
DateHistogramAggregationBuilder builder,
boolean usesFromRange
) throws IOException {
Expand All @@ -1227,7 +1264,14 @@ private void aggregationImplementationChoiceTestCase(
agg.preCollection();
context.searcher().search(context.query(), agg);
InternalDateHistogram result = (InternalDateHistogram) agg.buildTopLevel();
assertThat(result.getBuckets().stream().map(InternalDateHistogram.Bucket::getKeyAsString).collect(toList()), equalTo(data));
result = (InternalDateHistogram) result.reduce(
org.elasticsearch.common.collect.List.of(result),
ReduceContext.forFinalReduction(context.bigArrays(), null, context.multiBucketConsumer(), PipelineTree.EMPTY)
);
assertThat(
result.getBuckets().stream().map(InternalDateHistogram.Bucket::getKeyAsString).collect(toList()),
equalTo(resultingBucketKeys)
);
}
}
}
Expand Down

0 comments on commit bcb8ca0

Please sign in to comment.