Skip to content

Commit 664ba0a

Browse files
committed
Fix the parent join aggregator test case (#60991)
The test was putting parent and child documents into different segments which is unrealistic and was causing errors. Closes #60980
1 parent ce9c5f0 commit 664ba0a

File tree

2 files changed

+15
-14
lines changed

2 files changed

+15
-14
lines changed

modules/parent-join/src/test/java/org/elasticsearch/join/aggregations/ChildrenToParentAggregatorTests.java

+10-12
Original file line numberDiff line numberDiff line change
@@ -32,20 +32,19 @@
3232
import org.apache.lucene.search.TermInSetQuery;
3333
import org.apache.lucene.store.Directory;
3434
import org.apache.lucene.util.BytesRef;
35-
import org.apache.lucene.util.LuceneTestCase;
3635
import org.elasticsearch.Version;
3736
import org.elasticsearch.cluster.metadata.IndexMetadata;
3837
import org.elasticsearch.common.collect.Tuple;
3938
import org.elasticsearch.common.lucene.index.ElasticsearchDirectoryReader;
4039
import org.elasticsearch.common.settings.Settings;
4140
import org.elasticsearch.index.Index;
4241
import org.elasticsearch.index.mapper.ContentPath;
43-
import org.elasticsearch.index.mapper.MappingLookup;
4442
import org.elasticsearch.index.mapper.DocumentMapper;
4543
import org.elasticsearch.index.mapper.IdFieldMapper;
4644
import org.elasticsearch.index.mapper.MappedFieldType;
4745
import org.elasticsearch.index.mapper.Mapper;
4846
import org.elasticsearch.index.mapper.MapperService;
47+
import org.elasticsearch.index.mapper.MappingLookup;
4948
import org.elasticsearch.index.mapper.NumberFieldMapper;
5049
import org.elasticsearch.index.mapper.Uid;
5150
import org.elasticsearch.index.shard.ShardId;
@@ -75,7 +74,6 @@
7574
import static org.mockito.Mockito.mock;
7675
import static org.mockito.Mockito.when;
7776

78-
@LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/60980")
7977
public class ChildrenToParentAggregatorTests extends AggregatorTestCase {
8078

8179
private static final String CHILD_TYPE = "child_type";
@@ -121,9 +119,11 @@ public void testParentChild() throws IOException {
121119
expectedTotalParents++;
122120
expectedMinValue = Math.min(expectedMinValue, expectedValues.v2());
123121
}
124-
assertEquals("Having " + parent.getDocCount() + " docs and aggregation results: " +
125-
parent.getAggregations().asMap(),
126-
expectedTotalParents, parent.getDocCount());
122+
assertEquals(
123+
"Having " + parent.getDocCount() + " docs and aggregation results: " + parent,
124+
expectedTotalParents,
125+
parent.getDocCount()
126+
);
127127
assertEquals(expectedMinValue, ((InternalMin) parent.getAggregations().get("in_parent")).getValue(), Double.MIN_VALUE);
128128
assertTrue(JoinAggregationInspectionHelper.hasValue(parent));
129129
});
@@ -235,21 +235,19 @@ private static Map<String, Tuple<Integer, Integer>> setupIndex(RandomIndexWriter
235235
Map<String, Tuple<Integer, Integer>> expectedValues = new HashMap<>();
236236
int numParents = randomIntBetween(1, 10);
237237
for (int i = 0; i < numParents; i++) {
238+
List<List<Field>> documents = new ArrayList<>();
238239
String parent = "parent" + i;
239240
int randomValue = randomIntBetween(0, 100);
240-
List<Field> parentDocument = createParentDocument(parent, randomValue);
241-
/*long parentDocId =*/ iw.addDocument(parentDocument);
242-
//System.out.println("Parent: " + parent + ": " + randomValue + ", id: " + parentDocId);
241+
documents.add(createParentDocument(parent, randomValue));
243242
int numChildren = randomIntBetween(1, 10);
244243
int minValue = Integer.MAX_VALUE;
245244
for (int c = 0; c < numChildren; c++) {
246245
minValue = Math.min(minValue, randomValue);
247246
int randomSubValue = randomIntBetween(0, 100);
248-
List<Field> childDocument = createChildDocument("child" + c + "_" + parent, parent, randomSubValue);
249-
/*long childDocId =*/ iw.addDocument(childDocument);
250-
//System.out.println("Child: " + "child" + c + "_" + parent + ": " + randomSubValue + ", id: " + childDocId);
247+
documents.add(createChildDocument("child" + c + "_" + parent, parent, randomSubValue));
251248
}
252249
expectedValues.put(parent, new Tuple<>(numChildren, minValue));
250+
iw.addDocuments(documents);
253251
}
254252
return expectedValues;
255253
}

test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -424,9 +424,12 @@ protected <A extends InternalAggregation, C extends Aggregator> A searchAndReduc
424424
}
425425

426426
/**
427-
* Divides the provided {@link IndexSearcher} in sub-searcher, one for each segment,
428-
* builds an aggregator for each sub-searcher filtered by the provided {@link Query} and
427+
* Collects all documents that match the provided query {@link Query} and
429428
* returns the reduced {@link InternalAggregation}.
429+
* <p>
430+
* Half the time it aggregates each leaf individually and reduces all
431+
* results together. The other half the time it aggregates across the entire
432+
* index at once and runs a final reduction on the single resulting agg.
430433
*/
431434
protected <A extends InternalAggregation, C extends Aggregator> A searchAndReduce(IndexSettings indexSettings,
432435
IndexSearcher searcher,

0 commit comments

Comments
 (0)