Skip to content

Commit d331afb

Browse files
committed
Fix the parent join aggregator test case
The test was putting parent and child documents into different segments which is unrealistic and was causing errors. Closes elastic#60980
1 parent 766177f commit d331afb

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

Lines changed: 10 additions & 12 deletions
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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,9 +419,12 @@ protected <A extends InternalAggregation, C extends Aggregator> A searchAndReduc
419419
}
420420

421421
/**
422-
* Divides the provided {@link IndexSearcher} in sub-searcher, one for each segment,
423-
* builds an aggregator for each sub-searcher filtered by the provided {@link Query} and
422+
* Collects all documents that match the provided query {@link Query} and
424423
* returns the reduced {@link InternalAggregation}.
424+
* <p>
425+
* Half the time it aggregates each leaf individually and reduces all
426+
* results together. The other half the time it aggregates across the entire
427+
* index at once and runs a final reduction on the single resulting agg.
425428
*/
426429
protected <A extends InternalAggregation, C extends Aggregator> A searchAndReduce(IndexSettings indexSettings,
427430
IndexSearcher searcher,

0 commit comments

Comments
 (0)