Skip to content

Commit 8d58334

Browse files
committed
Cache the score of the parent document in the nested agg (#36019)
The nested agg can defer the collection of children if it is nested under another aggregation. In such case accessing the score in the children aggregation throws an error because the scorer has already advanced to the next parent. This change fixes this error by caching the score of the parent in the nested aggregation. Children aggregations that work on nested documents will be able to access the _score. Also note that the _score in this case is always the parent's score, there is no way to retrieve the score of a nested docs in aggregations. Closes #35985 Closes #34555
1 parent 06adb19 commit 8d58334

File tree

2 files changed

+68
-15
lines changed

2 files changed

+68
-15
lines changed

server/src/main/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregator.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,9 @@ class BufferingNestedLeafBucketCollector extends LeafBucketCollectorBase {
140140
final DocIdSetIterator childDocs;
141141
final LongArrayList bucketBuffer = new LongArrayList();
142142

143+
Scorer scorer;
143144
int currentParentDoc = -1;
145+
final CachedScorer cachedScorer = new CachedScorer();
144146

145147
BufferingNestedLeafBucketCollector(LeafBucketCollector sub, BitSet parentDocs, DocIdSetIterator childDocs) {
146148
super(sub, null);
@@ -149,6 +151,12 @@ class BufferingNestedLeafBucketCollector extends LeafBucketCollectorBase {
149151
this.childDocs = childDocs;
150152
}
151153

154+
@Override
155+
public void setScorer(Scorer scorer) throws IOException {
156+
this.scorer = scorer;
157+
super.setScorer(cachedScorer);
158+
}
159+
152160
@Override
153161
public void collect(int parentDoc, long bucket) throws IOException {
154162
// if parentDoc is 0 then this means that this parent doesn't have child docs (b/c these appear always before the parent
@@ -159,7 +167,12 @@ public void collect(int parentDoc, long bucket) throws IOException {
159167

160168
if (currentParentDoc != parentDoc) {
161169
processBufferedChildBuckets();
170+
if (needsScores()) {
171+
// cache the score of the current parent
172+
cachedScorer.score = scorer.score();
173+
}
162174
currentParentDoc = parentDoc;
175+
163176
}
164177
bucketBuffer.add(bucket);
165178
}
@@ -177,6 +190,7 @@ void processBufferedChildBuckets() throws IOException {
177190
}
178191

179192
for (; childDocId < currentParentDoc; childDocId = childDocs.nextDoc()) {
193+
cachedScorer.doc = childDocId;
180194
final long[] buffer = bucketBuffer.buffer;
181195
final int size = bucketBuffer.size();
182196
for (int i = 0; i < size; i++) {
@@ -185,6 +199,26 @@ void processBufferedChildBuckets() throws IOException {
185199
}
186200
bucketBuffer.clear();
187201
}
202+
}
203+
204+
private static class CachedScorer extends Scorer {
205+
int doc;
206+
float score;
207+
208+
private CachedScorer() { super(null); }
209+
210+
@Override
211+
public DocIdSetIterator iterator() {
212+
throw new UnsupportedOperationException();
213+
}
214+
215+
@Override
216+
public final float score() { return score; }
217+
218+
@Override
219+
public int docID() {
220+
return doc;
221+
}
188222

189223
}
190224

server/src/test/java/org/elasticsearch/search/aggregations/bucket/terms/TermsAggregatorTests.java

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import org.elasticsearch.index.mapper.SeqNoFieldMapper;
4848
import org.elasticsearch.index.mapper.TypeFieldMapper;
4949
import org.elasticsearch.index.mapper.UidFieldMapper;
50+
import org.elasticsearch.index.query.MatchAllQueryBuilder;
5051
import org.elasticsearch.index.query.QueryBuilders;
5152
import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
5253
import org.elasticsearch.search.SearchHit;
@@ -60,6 +61,7 @@
6061
import org.elasticsearch.search.aggregations.bucket.MultiBucketsAggregation;
6162
import org.elasticsearch.search.aggregations.bucket.filter.Filter;
6263
import org.elasticsearch.search.aggregations.bucket.filter.FilterAggregationBuilder;
64+
import org.elasticsearch.search.aggregations.bucket.filter.InternalFilter;
6365
import org.elasticsearch.search.aggregations.bucket.global.GlobalAggregationBuilder;
6466
import org.elasticsearch.search.aggregations.bucket.global.InternalGlobal;
6567
import org.elasticsearch.search.aggregations.bucket.nested.InternalNested;
@@ -1042,21 +1044,23 @@ public void testWithNestedAggregations() throws IOException {
10421044
fieldType.setHasDocValues(true);
10431045
fieldType.setName("nested_value");
10441046
try (IndexReader indexReader = wrap(DirectoryReader.open(directory))) {
1045-
InternalNested result = search(newSearcher(indexReader, false, true),
1046-
// match root document only
1047-
new DocValuesFieldExistsQuery(PRIMARY_TERM_NAME), nested, fieldType);
1048-
InternalMultiBucketAggregation<?, ?> terms = result.getAggregations().get("terms");
1049-
assertThat(terms.getBuckets().size(), equalTo(9));
1050-
int ptr = 9;
1051-
for (MultiBucketsAggregation.Bucket bucket : terms.getBuckets()) {
1052-
InternalTopHits topHits = bucket.getAggregations().get("top_hits");
1053-
assertThat(topHits.getHits().totalHits, equalTo((long) ptr));
1054-
if (withScore) {
1055-
assertThat(topHits.getHits().getMaxScore(), equalTo(1f));
1056-
} else {
1057-
assertThat(topHits.getHits().getMaxScore(), equalTo(Float.NaN));
1058-
}
1059-
--ptr;
1047+
{
1048+
InternalNested result = search(newSearcher(indexReader, false, true),
1049+
// match root document only
1050+
new DocValuesFieldExistsQuery(PRIMARY_TERM_NAME), nested, fieldType);
1051+
InternalMultiBucketAggregation<?, ?> terms = result.getAggregations().get("terms");
1052+
assertNestedTopHitsScore(terms, withScore);
1053+
}
1054+
1055+
{
1056+
FilterAggregationBuilder filter = new FilterAggregationBuilder("filter", new MatchAllQueryBuilder())
1057+
.subAggregation(nested);
1058+
InternalFilter result = search(newSearcher(indexReader, false, true),
1059+
// match root document only
1060+
new DocValuesFieldExistsQuery(PRIMARY_TERM_NAME), filter, fieldType);
1061+
InternalNested nestedResult = result.getAggregations().get("nested");
1062+
InternalMultiBucketAggregation<?, ?> terms = nestedResult.getAggregations().get("terms");
1063+
assertNestedTopHitsScore(terms, withScore);
10601064
}
10611065
}
10621066
}
@@ -1065,6 +1069,21 @@ public void testWithNestedAggregations() throws IOException {
10651069
}
10661070
}
10671071

1072+
private void assertNestedTopHitsScore(InternalMultiBucketAggregation<?, ?> terms, boolean withScore) {
1073+
assertThat(terms.getBuckets().size(), equalTo(9));
1074+
int ptr = 9;
1075+
for (MultiBucketsAggregation.Bucket bucket : terms.getBuckets()) {
1076+
InternalTopHits topHits = bucket.getAggregations().get("top_hits");
1077+
assertThat(topHits.getHits().totalHits, equalTo((long) ptr));
1078+
if (withScore) {
1079+
assertThat(topHits.getHits().getMaxScore(), equalTo(1f));
1080+
} else {
1081+
assertThat(topHits.getHits().getMaxScore(), equalTo(Float.NaN));
1082+
}
1083+
--ptr;
1084+
}
1085+
}
1086+
10681087
private final SeqNoFieldMapper.SequenceIDFields sequenceIDFields = SeqNoFieldMapper.SequenceIDFields.emptySeqID();
10691088
private List<Document> generateDocsWithNested(String id, int value, int[] nestedValues) {
10701089
List<Document> documents = new ArrayList<>();

0 commit comments

Comments
 (0)