Skip to content

Commit 0597530

Browse files
committed
Fix needsScore computation in GlobalOrdCardinalityAggregator (elastic#113129)
Only use TOP_DOCS if we are going to use dynamic pruning.
1 parent 422cf26 commit 0597530

File tree

3 files changed

+100
-1
lines changed

3 files changed

+100
-1
lines changed

docs/changelog/113129.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 113129
2+
summary: Fix `needsScore` computation in `GlobalOrdCardinalityAggregator`
3+
area: Aggregations
4+
type: bug
5+
issues:
6+
- 112975

server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/NestedIT.java

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
package org.elasticsearch.search.aggregations.bucket;
99

1010
import org.apache.lucene.search.join.ScoreMode;
11+
import org.elasticsearch.action.admin.indices.delete.DeleteIndexRequest;
1112
import org.elasticsearch.action.index.IndexRequestBuilder;
1213
import org.elasticsearch.action.search.SearchRequestBuilder;
1314
import org.elasticsearch.index.query.InnerHitBuilder;
@@ -17,11 +18,15 @@
1718
import org.elasticsearch.search.aggregations.InternalAggregation;
1819
import org.elasticsearch.search.aggregations.bucket.filter.Filter;
1920
import org.elasticsearch.search.aggregations.bucket.histogram.Histogram;
21+
import org.elasticsearch.search.aggregations.bucket.nested.InternalNested;
2022
import org.elasticsearch.search.aggregations.bucket.nested.Nested;
23+
import org.elasticsearch.search.aggregations.bucket.nested.NestedAggregationBuilder;
2124
import org.elasticsearch.search.aggregations.bucket.terms.LongTerms;
2225
import org.elasticsearch.search.aggregations.bucket.terms.StringTerms;
2326
import org.elasticsearch.search.aggregations.bucket.terms.Terms;
2427
import org.elasticsearch.search.aggregations.bucket.terms.Terms.Bucket;
28+
import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder;
29+
import org.elasticsearch.search.aggregations.metrics.CardinalityAggregationBuilder;
2530
import org.elasticsearch.search.aggregations.metrics.Max;
2631
import org.elasticsearch.search.aggregations.metrics.Stats;
2732
import org.elasticsearch.search.aggregations.metrics.Sum;
@@ -889,4 +894,87 @@ public void testSyntheticSource() throws Exception {
889894
assertEquals("a", nested.get("number"));
890895
});
891896
}
897+
898+
public void testScoring() throws Exception {
899+
assertAcked(
900+
prepareCreate("scoring").setMapping(
901+
jsonBuilder().startObject()
902+
.startObject("properties")
903+
.startObject("tags")
904+
.field("type", "nested")
905+
.startObject("properties")
906+
.startObject("key")
907+
.field("type", "keyword")
908+
.endObject()
909+
.startObject("value")
910+
.field("type", "keyword")
911+
.endObject()
912+
.endObject()
913+
.endObject()
914+
.endObject()
915+
.endObject()
916+
)
917+
);
918+
ensureGreen("scoring");
919+
920+
prepareIndex("scoring").setId("1")
921+
.setSource(
922+
jsonBuilder().startObject()
923+
.startArray("tags")
924+
.startObject()
925+
.field("key", "state")
926+
.field("value", "texas")
927+
.endObject()
928+
.endArray()
929+
.endObject()
930+
)
931+
.get();
932+
refresh("scoring");
933+
prepareIndex("scoring").setId("2")
934+
.setSource(
935+
jsonBuilder().startObject()
936+
.startArray("tags")
937+
.startObject()
938+
.field("key", "state")
939+
.field("value", "utah")
940+
.endObject()
941+
.endArray()
942+
.endObject()
943+
)
944+
.get();
945+
refresh("scoring");
946+
prepareIndex("scoring").setId("3")
947+
.setSource(
948+
jsonBuilder().startObject()
949+
.startArray("tags")
950+
.startObject()
951+
.field("key", "state")
952+
.field("value", "texas")
953+
.endObject()
954+
.endArray()
955+
.endObject()
956+
)
957+
.get();
958+
refresh("scoring");
959+
960+
assertResponse(
961+
client().prepareSearch("scoring")
962+
.setSize(0)
963+
.addAggregation(
964+
new NestedAggregationBuilder("tags", "tags").subAggregation(
965+
new TermsAggregationBuilder("keys").field("tags.key")
966+
.executionHint("map")
967+
.subAggregation(new TermsAggregationBuilder("values").field("tags.value"))
968+
.subAggregation(new CardinalityAggregationBuilder("values_count").field("tags.value"))
969+
)
970+
),
971+
searchResponse -> {
972+
InternalNested nested = searchResponse.getAggregations().get("tags");
973+
assertThat(nested.getDocCount(), equalTo(3L));
974+
assertThat(nested.getAggregations().asList().size(), equalTo(1));
975+
}
976+
);
977+
978+
assertAcked(indicesAdmin().delete(new DeleteIndexRequest("scoring")).get());
979+
}
892980
}

server/src/main/java/org/elasticsearch/search/aggregations/metrics/GlobalOrdCardinalityAggregator.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,12 @@ public GlobalOrdCardinalityAggregator(
9292

9393
@Override
9494
public ScoreMode scoreMode() {
95-
if (field != null && valuesSource.needsScores() == false && maxOrd <= MAX_FIELD_CARDINALITY_FOR_DYNAMIC_PRUNING) {
95+
// this check needs to line up with the dynamic pruning as it is the
96+
// only case where TOP_DOCS make sense.branch
97+
if (this.parent == null
98+
&& field != null
99+
&& valuesSource.needsScores() == false
100+
&& maxOrd <= MAX_FIELD_CARDINALITY_FOR_DYNAMIC_PRUNING) {
96101
return ScoreMode.TOP_DOCS;
97102
} else if (valuesSource.needsScores()) {
98103
return ScoreMode.COMPLETE;

0 commit comments

Comments
 (0)