Skip to content

Commit

Permalink
Change InternalSignificantTerms to only sum shard level counts in fin…
Browse files Browse the repository at this point in the history
…al reduce

Signed-off-by: Jay Deng <jayd0104@gmail.com>
  • Loading branch information
jed326 authored and Jay Deng committed Jul 17, 2023
1 parent 8bd6b7c commit a3a09e3
Showing 1 changed file with 12 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,12 @@ public InternalAggregation reduce(List<InternalAggregation> aggregations, Reduce
@SuppressWarnings("unchecked")
InternalSignificantTerms<A, B> terms = (InternalSignificantTerms<A, B>) aggregation;
globalSubsetSize += terms.getSubsetSize();
globalSupersetSize += terms.getSupersetSize();
// supersetSize is a shard level count, so we only sum on the final reduce
if (reduceContext.isFinalReduce()) {
globalSupersetSize += terms.getSupersetSize(); // This can't be summed for slices
} else {
globalSupersetSize = terms.getSupersetSize(); // This can't be summed for slices

Check warning on line 239 in server/src/main/java/org/opensearch/search/aggregations/bucket/terms/InternalSignificantTerms.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/aggregations/bucket/terms/InternalSignificantTerms.java#L239

Added line #L239 was not covered by tests
}
}
Map<String, List<B>> buckets = new HashMap<>();
for (InternalAggregation aggregation : aggregations) {
Expand Down Expand Up @@ -291,7 +296,12 @@ protected B reduceBucket(List<B> buckets, ReduceContext context) {
List<InternalAggregations> aggregationsList = new ArrayList<>(buckets.size());
for (B bucket : buckets) {
subsetDf += bucket.subsetDf;
supersetDf += bucket.supersetDf;
// supersetDf is a shard level count, so we only sum on the final reduce
if (context.isFinalReduce()) {
supersetDf += bucket.supersetDf;
} else {
supersetDf = bucket.supersetDf;

Check warning on line 303 in server/src/main/java/org/opensearch/search/aggregations/bucket/terms/InternalSignificantTerms.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/aggregations/bucket/terms/InternalSignificantTerms.java#L303

Added line #L303 was not covered by tests
}
aggregationsList.add(bucket.aggregations);
}
InternalAggregations aggs = InternalAggregations.reduce(aggregationsList, context);
Expand Down

0 comments on commit a3a09e3

Please sign in to comment.