Skip to content

Commit 86a23cb

Browse files
committed
tests pass, I think
Signed-off-by: Anthony Leong <aj.leong623@gmail.com>
1 parent 0375104 commit 86a23cb

File tree

2 files changed

+10
-10
lines changed

2 files changed

+10
-10
lines changed

server/src/main/java/org/opensearch/search/aggregations/bucket/missing/MissingAggregator.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -123,17 +123,17 @@ protected boolean tryPrecomputeAggregationForLeaf(LeafReaderContext ctx) throws
123123
return false;
124124
}
125125

126-
// There is a chance that when the fieldName does not exist, there could be a missing
127-
// parameter. We must check that case separately.
126+
// When fieldname does not exist, we cannot collect through the precomputation.
128127
if (fieldName == null || weight == null) {
129-
// we do not collect any documents through the missing aggregation when the missing parameter
130-
// is up.
131-
if (valuesSourceConfig != null && valuesSourceConfig.missing() != null) {
132-
return true;
133-
}
134128
return false;
135129
}
136130

131+
// we do not collect any documents through the missing aggregation when the missing parameter
132+
// is up.
133+
if (valuesSourceConfig != null && valuesSourceConfig.missing() != null) {
134+
return true;
135+
}
136+
137137
// The optimization could only be used if there are no deleted documents and the top-level
138138
// query matches all documents in the segment.
139139
if (weight.count(ctx) == 0) {

server/src/test/java/org/opensearch/search/aggregations/bucket/missing/MissingAggregatorTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,7 @@ public void testUnmappedWithMissingParam() throws IOException {
452452

453453
final MissingAggregationBuilder builder = new MissingAggregationBuilder("_name").field("unknown_field").missing(randomLong());
454454

455-
// Having the missing parameter will make the missing aggregator not responsible for any documents, so it will short circuit
455+
// Because the field is unmapped, the fieldname will not existm so we cannot collect through precomputation optimization.
456456
testCase(newMatchAllQuery(), builder, writer -> {
457457
for (int i = 0; i < numDocs; i++) {
458458
final long randomLong = randomLong();
@@ -466,7 +466,7 @@ public void testUnmappedWithMissingParam() throws IOException {
466466
}, internalMissing -> {
467467
assertEquals(0, internalMissing.getDocCount());
468468
assertFalse(AggregationInspectionHelper.hasValue(internalMissing));
469-
}, singleton(aggFieldType), 0);
469+
}, singleton(aggFieldType), numDocs);
470470

471471
testCase(newMatchAllQuery(), builder, writer -> {
472472
for (int i = 0; i < numDocs; i++) {
@@ -475,7 +475,7 @@ public void testUnmappedWithMissingParam() throws IOException {
475475
}, internalMissing -> {
476476
assertEquals(0, internalMissing.getDocCount());
477477
assertFalse(AggregationInspectionHelper.hasValue(internalMissing));
478-
}, singleton(aggFieldType), 0);
478+
}, singleton(aggFieldType), numDocs);
479479
}
480480

481481
public void testMissingParam() throws IOException {

0 commit comments

Comments
 (0)