From 1fc78d430b47895159987487f5571d241256a4aa Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Tue, 22 Sep 2020 00:02:22 +0200 Subject: [PATCH] Fix terms aggregation ordering after the final reduce (#62732) This commit ensures that the final order of the terms aggregations is registered correctly after the final reduce. This bug was introduced in #62028 which is not released yet so this PR is marked as a non-issue. This issue was discovered when running a terms aggregation under an auto-date histogram. In such a case, the auto-date histogram may run multiple final reduce to merge buckets together. This change makes sure that running multiple final reduces doesn't create duplicates but it doesn't fix the fact that the final reduce may prune the list of terms prematurely. This other bug is tracked separately in #62731. --- .../search/aggregations/bucket/terms/InternalTerms.java | 2 +- .../aggregations/bucket/terms/InternalTermsTestCase.java | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java index 376e9b9b566..cd6f3b0afb9 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTerms.java @@ -426,7 +426,7 @@ public abstract class InternalTerms, B extends Int } else { docCountError = aggregations.size() == 1 ? 0 : sumDocCountError; } - return create(name, Arrays.asList(list), thisReduceOrder, docCountError, otherDocCount); + return create(name, Arrays.asList(list), reduceContext.isFinalReduce() ? order : thisReduceOrder, docCountError, otherDocCount); } @Override diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTermsTestCase.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTermsTestCase.java index 11dbd522e8c..6c1063ffbe3 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTermsTestCase.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/InternalTermsTestCase.java @@ -31,6 +31,8 @@ import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.Stream; +import static org.hamcrest.Matchers.equalTo; + public abstract class InternalTermsTestCase extends InternalMultiBucketAggregationTestCase> { private boolean showDocCount; @@ -88,6 +90,10 @@ public abstract class InternalTermsTestCase extends InternalMultiBucketAggregati final long expectedTotalDocCount = inputs.stream().map(Terms::getBuckets) .flatMap(List::stream).mapToLong(Terms.Bucket::getDocCount).sum(); assertEquals(expectedTotalDocCount, reducedTotalDocCount); + for (InternalTerms terms : inputs) { + assertThat(reduced.reduceOrder, equalTo(terms.order)); + assertThat(reduced.order, equalTo(terms.order)); + } } private static Map toCounts(Stream buckets) {