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

Fix docCountError calculation for multiple reduces #76391

Merged
merged 2 commits into from
Aug 12, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ private StringTerms newTerms(Random rand, BytesRef[] dict, boolean withNested) {
true,
0,
buckets,
0L
null
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ private StringTerms newTerms(boolean withNested) {
false,
100000,
resultBuckets,
0L
null
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1034,7 +1034,6 @@ public void testFixedDocs() throws Exception {
* Tests the upper bounds are correct when performing incremental reductions
* See https://github.com/elastic/elasticsearch/issues/40005 for more details
*/
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/75667")
public void testIncrementalReduction() {
SearchResponse response = client().prepareSearch("idx_fixed_docs_3", "idx_fixed_docs_4", "idx_fixed_docs_5")
.addAggregation(terms("terms")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ private long getDocCountError(A terms) {
if (size == 0 || size < terms.getShardSize() || isKeyOrder(terms.getOrder())) {
return 0;
} else if (InternalOrder.isCountDesc(terms.getOrder())) {
if (terms.getDocCountError() != null && terms.getDocCountError() > 0) {
if (terms.getDocCountError() != null) {
// If there is an existing docCountError for this agg then
// use this as the error for this aggregation
return terms.getDocCountError();
Expand Down Expand Up @@ -340,7 +340,7 @@ public InternalAggregation reduce(List<InternalAggregation> aggregations, Intern

protected static XContentBuilder doXContentCommon(XContentBuilder builder,
Params params,
long docCountError,
Long docCountError,
long otherDocCount,
List<? extends AbstractTermsBucket> buckets) throws IOException {
builder.field(DOC_COUNT_ERROR_UPPER_BOUND_FIELD_NAME.getPreferredName(), docCountError);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,7 @@ StringTerms buildResult(long owningBucketOrd, long otherDocCount, StringTerms.Bu
}
return new StringTerms(name, reduceOrder, order, bucketCountThresholds.getRequiredSize(),
bucketCountThresholds.getMinDocCount(), metadata(), format, bucketCountThresholds.getShardSize(), showTermDocCountError,
otherDocCount, Arrays.asList(topBuckets), 0L);
otherDocCount, Arrays.asList(topBuckets), null);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,13 @@ protected InternalMappedTerms(String name, BucketOrder reduceOrder, BucketOrder
protected InternalMappedTerms(StreamInput in, Bucket.Reader<B> bucketReader) throws IOException {
super(in);
if (in.getVersion().onOrAfter(Version.V_8_0_0)) { // todo fix after backport
docCountError = in.readOptionalLong();
} else {
docCountError = in.readZLong();
if (docCountError == 0) {
if (in.readBoolean()) {
docCountError = in.readZLong();
} else {
docCountError = null;
}
} else {
docCountError = in.readZLong();
}
format = in.readNamedWriteable(DocValueFormat.class);
shardSize = readSize(in);
Expand All @@ -70,7 +71,12 @@ protected InternalMappedTerms(StreamInput in, Bucket.Reader<B> bucketReader) thr
@Override
protected final void writeTermTypeInfoTo(StreamOutput out) throws IOException {
if (out.getVersion().onOrAfter(Version.V_8_0_0)) { // todo fix after backport
out.writeOptionalLong(docCountError);
if (docCountError != null) {
out.writeBoolean(true);
out.writeZLong(docCountError);
} else {
out.writeBoolean(false);
}
} else {
out.writeZLong(docCountError == null ? 0 : docCountError);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ StringTerms buildResult(long owningBucketOrd, long otherDocCount, StringTerms.Bu
}
return new StringTerms(name, reduceOrder, order, bucketCountThresholds.getRequiredSize(),
bucketCountThresholds.getMinDocCount(), metadata(), format, bucketCountThresholds.getShardSize(), showTermDocCountError,
otherDocCount, Arrays.asList(topBuckets), 0L);
otherDocCount, Arrays.asList(topBuckets), null);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ LongTerms buildResult(long owningBucketOrd, long otherDocCount, LongTerms.Bucket
showTermDocCountError,
otherDocCount,
List.of(topBuckets),
0L
null
);
}

Expand Down Expand Up @@ -454,7 +454,7 @@ DoubleTerms buildResult(long owningBucketOrd, long otherDocCount, DoubleTerms.Bu
showTermDocCountError,
otherDocCount,
List.of(topBuckets),
0L
null
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ protected boolean lessThan(OrdBucket a, OrdBucket b) {
showTermDocCountError,
otherDocsCount,
buckets,
0L
null
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public boolean isMapped() {

@Override
public final XContentBuilder doXContentBody(XContentBuilder builder, Params params) throws IOException {
return doXContentCommon(builder, params, 0, 0, Collections.emptyList());
return doXContentCommon(builder, params, 0L, 0, Collections.emptyList());
}

@Override
Expand Down