From 1cbff6a1f2fdf10a04e2262779022b37b308e68e Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Fri, 29 Jan 2021 16:55:14 -0500 Subject: [PATCH 01/12] I am surprised this fails --- .../composite/CompositeAggregatorTests.java | 132 ++++++++++++++++++ .../bucket/nested/NestedAggregatorTests.java | 3 +- 2 files changed, 133 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java index 60ccb9a4562fc..8e1938ac7f824 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java @@ -57,15 +57,22 @@ import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.mapper.DateFieldMapper; import org.elasticsearch.index.mapper.GeoPointFieldMapper; +import org.elasticsearch.index.mapper.IdFieldMapper; import org.elasticsearch.index.mapper.IpFieldMapper; import org.elasticsearch.index.mapper.KeywordFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; +import org.elasticsearch.index.mapper.NestedPathFieldMapper; import org.elasticsearch.index.mapper.NumberFieldMapper; +import org.elasticsearch.index.mapper.ObjectMapper; +import org.elasticsearch.index.mapper.SeqNoFieldMapper; +import org.elasticsearch.index.mapper.Uid; import org.elasticsearch.search.aggregations.Aggregator; import org.elasticsearch.search.aggregations.AggregatorTestCase; +import org.elasticsearch.search.aggregations.bucket.InternalSingleBucketAggregation; import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileGridAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils; import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval; +import org.elasticsearch.search.aggregations.bucket.nested.NestedAggregationBuilder; import org.elasticsearch.search.aggregations.bucket.terms.StringTerms; import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder; import org.elasticsearch.search.aggregations.metrics.InternalMax; @@ -97,12 +104,14 @@ import java.util.function.Supplier; import java.util.stream.Collectors; +import static org.elasticsearch.search.aggregations.bucket.nested.NestedAggregatorTests.nestedObject; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; public class CompositeAggregatorTests extends AggregatorTestCase { private static MappedFieldType[] FIELD_TYPES; + private List objectMappers; @Override @Before @@ -117,6 +126,8 @@ public void setUp() throws Exception { FIELD_TYPES[5] = new KeywordFieldMapper.KeywordFieldType("terms"); FIELD_TYPES[6] = new IpFieldMapper.IpFieldType("ip"); FIELD_TYPES[7] = new GeoPointFieldMapper.GeoPointFieldType("geo_point"); + + objectMappers = new ArrayList<>(); } @Override @@ -124,6 +135,12 @@ public void setUp() throws Exception { public void tearDown() throws Exception { super.tearDown(); FIELD_TYPES = null; + objectMappers = null; + } + + @Override + protected List objectMappers() { + return objectMappers; } public void testUnmappedFieldWithTerms() throws Exception { @@ -489,6 +506,105 @@ public void testWithKeyword() throws Exception { ); } + /** + * This is just a template for migrating to the test case execution in {@link AggregatorTestCase}, it doesn't test anything new. + */ + public void testUsingTestCase() throws Exception { + TermsValuesSourceBuilder terms = new TermsValuesSourceBuilder("keyword").field("keyword"); + final List>> dataset = new ArrayList<>(); + dataset.addAll( + Arrays.asList( + createDocument("keyword", "a"), + createDocument("keyword", "c"), + createDocument("keyword", "a"), + createDocument("keyword", "d"), + createDocument("keyword", "c") + ) + ); + testCase( + new CompositeAggregationBuilder("name", Collections.singletonList(terms)), + new MatchAllDocsQuery(), + iw -> { + Document document = new Document(); + int id = 0; + for (Map> fields : dataset) { + document.clear(); + addToDocument(id, document, fields); + iw.addDocument(document); + id++; + } + }, + (InternalComposite result) -> { + assertEquals(3, result.getBuckets().size()); + assertEquals("{keyword=d}", result.afterKey().toString()); + assertEquals("{keyword=a}", result.getBuckets().get(0).getKeyAsString()); + assertEquals(2L, result.getBuckets().get(0).getDocCount()); + assertEquals("{keyword=c}", result.getBuckets().get(1).getKeyAsString()); + assertEquals(2L, result.getBuckets().get(1).getDocCount()); + assertEquals("{keyword=d}", result.getBuckets().get(2).getKeyAsString()); + assertEquals(1L, result.getBuckets().get(2).getDocCount()); + }, + FIELD_TYPES + ); + } + + public void testSubAggregationOfNested() throws Exception { + final String nestedPath = "sellers"; + objectMappers.add(nestedObject(nestedPath)); + SeqNoFieldMapper.SequenceIDFields sequenceIDFields = SeqNoFieldMapper.SequenceIDFields.emptySeqID(); + TermsValuesSourceBuilder terms = new TermsValuesSourceBuilder("keyword").field("name"); + NestedAggregationBuilder builder = new NestedAggregationBuilder("nestedAggName", nestedPath); + builder.subAggregation(new CompositeAggregationBuilder("compositeAggName", Collections.singletonList(terms))); + testCase( + builder, + new MatchAllDocsQuery(), + iw -> { + // Sub-Docs + List documents = new ArrayList<>(); + documents.add(createNestedDocument("1", nestedPath, createDocument("name", "Pens and Stuff", "price" , 10L))); + documents.add(createNestedDocument("1", nestedPath, createDocument("name", "Pen World", "price" , 9L))); + documents.add(createNestedDocument("2", nestedPath, createDocument("name", "Pens and Stuff", "price" , 5L))); + documents.add(createNestedDocument("2", nestedPath, createDocument("name", "Stationary", "price" , 7L))); + // Root docs + Document root; + root = new Document(); + root.add(new Field(IdFieldMapper.NAME, Uid.encodeId("1"), IdFieldMapper.Defaults.FIELD_TYPE)); + root.add(sequenceIDFields.primaryTerm); + root.add(new StringField("name", new BytesRef("Ballpoint"), Field.Store.NO)); + documents.add(root); + + root = new Document(); + root.add(new Field(IdFieldMapper.NAME, Uid.encodeId("2"), IdFieldMapper.Defaults.FIELD_TYPE)); + root.add(new StringField("name", new BytesRef("Notebook"), Field.Store.NO)); + root.add(sequenceIDFields.primaryTerm); + documents.add(root); + iw.addDocuments(documents); + }, + (InternalSingleBucketAggregation parent) -> { + assertEquals(1, parent.getAggregations().asList().size()); + InternalComposite result = (InternalComposite) parent.getProperty("compositeAggName"); + assertEquals(3, result.getBuckets().size()); + assertEquals("{keyword=Stationary}", result.afterKey().toString()); + assertEquals("{keyword=Pen World}", result.getBuckets().get(0).getKeyAsString()); + assertEquals(1L, result.getBuckets().get(0).getDocCount()); + assertEquals("{keyword=Pens and Stuff}", result.getBuckets().get(1).getKeyAsString()); + assertEquals(2L, result.getBuckets().get(1).getDocCount()); + assertEquals("{keyword=Stationary}", result.getBuckets().get(2).getKeyAsString()); + assertEquals(1L, result.getBuckets().get(2).getDocCount()); + }, + new KeywordFieldMapper.KeywordFieldType("name"), + new NumberFieldMapper.NumberFieldType("price", NumberFieldMapper.NumberType.LONG) + ); + } + + private Document createNestedDocument(String id, String nestedPath, Map> fields) { + Document doc = new Document(); + doc.add(new Field(IdFieldMapper.NAME, Uid.encodeId(id), IdFieldMapper.Defaults.NESTED_FIELD_TYPE)); + doc.add(new Field(NestedPathFieldMapper.NAME, nestedPath, NestedPathFieldMapper.Defaults.FIELD_TYPE)); + addToDocument(doc, fields); + return doc; + } + public void testWithKeywordAndMissingBucket() throws Exception { final List>> dataset = new ArrayList<>(); dataset.addAll( @@ -1959,6 +2075,18 @@ public void testNullSourceNonNullCollection() { assertThat(e.getMessage(), equalTo("Composite source cannot be null")); } + /** + * Composite aggregation cannot be a sub-aggregation of a terms aggregation + */ + public void testAsChildOfTerms() { + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> { + executeTestCase(false, false, new MatchAllDocsQuery(), List.of(), + () -> {return null;}, + (c) -> {}); + }); + } + + private , V extends Comparable> void testRandomTerms(String field, Supplier randomSupplier, Function transformKey) throws IOException { @@ -2302,6 +2430,10 @@ private static IndexSettings createIndexSettings(Sort sort) { private void addToDocument(int id, Document doc, Map> keys) { doc.add(new StringField("id", Integer.toString(id), Field.Store.NO)); + addToDocument(doc, keys); + } + + private void addToDocument(Document doc, Map> keys) { for (Map.Entry> entry : keys.entrySet()) { final String name = entry.getKey(); for (Object value : entry.getValue()) { diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregatorTests.java index c043a77e55d54..2eedf252cc11e 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregatorTests.java @@ -172,8 +172,7 @@ public void testSingleNestingMax() throws IOException { Document document = new Document(); document.add(new Field(IdFieldMapper.NAME, Uid.encodeId(Integer.toString(i)), IdFieldMapper.Defaults.FIELD_TYPE)); - document.add(new Field(NestedPathFieldMapper.NAME, "test", - NestedPathFieldMapper.Defaults.FIELD_TYPE)); + document.add(new Field(NestedPathFieldMapper.NAME, "test", NestedPathFieldMapper.Defaults.FIELD_TYPE)); document.add(sequenceIDFields.primaryTerm); documents.add(document); iw.addDocuments(documents); From a244b2405c1357605c56793a75fefcb09c63b2a3 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Fri, 29 Jan 2021 17:21:04 -0500 Subject: [PATCH 02/12] Removed stub of a test I didn't end up using --- .../bucket/composite/CompositeAggregatorTests.java | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java index 8e1938ac7f824..ad7b9f1871f3d 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java @@ -2075,18 +2075,6 @@ public void testNullSourceNonNullCollection() { assertThat(e.getMessage(), equalTo("Composite source cannot be null")); } - /** - * Composite aggregation cannot be a sub-aggregation of a terms aggregation - */ - public void testAsChildOfTerms() { - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> { - executeTestCase(false, false, new MatchAllDocsQuery(), List.of(), - () -> {return null;}, - (c) -> {}); - }); - } - - private , V extends Comparable> void testRandomTerms(String field, Supplier randomSupplier, Function transformKey) throws IOException { From a9ba2b0f2eedff60323e85eb0c16b94dfab74ae0 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Thu, 4 Feb 2021 09:33:21 -0500 Subject: [PATCH 03/12] Add nested-specific directory wrapping logic --- .../index/cache/bitset/BitsetFilterCache.java | 4 ++++ .../bucket/composite/CompositeAggregatorTests.java | 13 ++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/cache/bitset/BitsetFilterCache.java b/server/src/main/java/org/elasticsearch/index/cache/bitset/BitsetFilterCache.java index 9a2dab81acf9b..5a9a9f2c3fcc7 100644 --- a/server/src/main/java/org/elasticsearch/index/cache/bitset/BitsetFilterCache.java +++ b/server/src/main/java/org/elasticsearch/index/cache/bitset/BitsetFilterCache.java @@ -135,6 +135,10 @@ private BitSet getAndLoadIfNotPresent(final Query query, final LeafReaderContext } final IndexReader.CacheKey coreCacheReader = cacheHelper.getKey(); final ShardId shardId = ShardUtils.extractShardId(context.reader()); + if (shardId == null) { + throw new IllegalStateException("Null shardId. If you got here from a test, you need to wrap the directory reader. " + + "see for example AggregatorTestCase#wrapInMockESDirectoryReader. If you got here in production, please file a bug."); + } if (indexSettings.getIndex().equals(shardId.getIndex()) == false) { // insanity throw new IllegalStateException("Trying to load bit set for index " + shardId.getIndex() diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java index ad7b9f1871f3d..0d3fd72eac332 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java @@ -112,6 +112,7 @@ public class CompositeAggregatorTests extends AggregatorTestCase { private static MappedFieldType[] FIELD_TYPES; private List objectMappers; + private boolean useNestedDirectoryWrapping = false; @Override @Before @@ -136,6 +137,7 @@ public void tearDown() throws Exception { super.tearDown(); FIELD_TYPES = null; objectMappers = null; + useNestedDirectoryWrapping = false; } @Override @@ -143,6 +145,14 @@ protected List objectMappers() { return objectMappers; } + @Override + protected IndexReader wrapDirectoryReader(DirectoryReader reader) throws IOException { + if (useNestedDirectoryWrapping) { + return wrapInMockESDirectoryReader(reader); + } + return reader; + } + public void testUnmappedFieldWithTerms() throws Exception { final List>> dataset = new ArrayList<>(); @@ -551,8 +561,9 @@ public void testUsingTestCase() throws Exception { public void testSubAggregationOfNested() throws Exception { final String nestedPath = "sellers"; objectMappers.add(nestedObject(nestedPath)); + useNestedDirectoryWrapping = true; SeqNoFieldMapper.SequenceIDFields sequenceIDFields = SeqNoFieldMapper.SequenceIDFields.emptySeqID(); - TermsValuesSourceBuilder terms = new TermsValuesSourceBuilder("keyword").field("name"); + TermsValuesSourceBuilder terms = new TermsValuesSourceBuilder("keyword").field(nestedPath + ".name"); NestedAggregationBuilder builder = new NestedAggregationBuilder("nestedAggName", nestedPath); builder.subAggregation(new CompositeAggregationBuilder("compositeAggName", Collections.singletonList(terms))); testCase( From f364a19e8281129d598df27a530b08f07e14ad83 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Thu, 4 Feb 2021 11:37:14 -0500 Subject: [PATCH 04/12] test passes, but why? --- .../composite/CompositeAggregatorTests.java | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java index 0d3fd72eac332..39408a5bfbc05 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java @@ -563,7 +563,10 @@ public void testSubAggregationOfNested() throws Exception { objectMappers.add(nestedObject(nestedPath)); useNestedDirectoryWrapping = true; SeqNoFieldMapper.SequenceIDFields sequenceIDFields = SeqNoFieldMapper.SequenceIDFields.emptySeqID(); - TermsValuesSourceBuilder terms = new TermsValuesSourceBuilder("keyword").field(nestedPath + ".name"); + final String leafNameField = "name"; + final String rootNameField = "product"; + // TermsValuesSourceBuilder terms = new TermsValuesSourceBuilder("keyword").field(nestedPath + ".name"); + TermsValuesSourceBuilder terms = new TermsValuesSourceBuilder("keyword").field(leafNameField); NestedAggregationBuilder builder = new NestedAggregationBuilder("nestedAggName", nestedPath); builder.subAggregation(new CompositeAggregationBuilder("compositeAggName", Collections.singletonList(terms))); testCase( @@ -572,21 +575,21 @@ public void testSubAggregationOfNested() throws Exception { iw -> { // Sub-Docs List documents = new ArrayList<>(); - documents.add(createNestedDocument("1", nestedPath, createDocument("name", "Pens and Stuff", "price" , 10L))); - documents.add(createNestedDocument("1", nestedPath, createDocument("name", "Pen World", "price" , 9L))); - documents.add(createNestedDocument("2", nestedPath, createDocument("name", "Pens and Stuff", "price" , 5L))); - documents.add(createNestedDocument("2", nestedPath, createDocument("name", "Stationary", "price" , 7L))); + documents.add(createNestedDocument("1", nestedPath, createDocument(leafNameField, "Pens and Stuff", "price" , 10L))); + documents.add(createNestedDocument("1", nestedPath, createDocument(leafNameField, "Pen World", "price" , 9L))); + documents.add(createNestedDocument("2", nestedPath, createDocument(leafNameField, "Pens and Stuff", "price" , 5L))); + documents.add(createNestedDocument("2", nestedPath, createDocument(leafNameField, "Stationary", "price" , 7L))); // Root docs Document root; root = new Document(); root.add(new Field(IdFieldMapper.NAME, Uid.encodeId("1"), IdFieldMapper.Defaults.FIELD_TYPE)); root.add(sequenceIDFields.primaryTerm); - root.add(new StringField("name", new BytesRef("Ballpoint"), Field.Store.NO)); + root.add(new StringField(rootNameField, new BytesRef("Ballpoint"), Field.Store.NO)); documents.add(root); root = new Document(); root.add(new Field(IdFieldMapper.NAME, Uid.encodeId("2"), IdFieldMapper.Defaults.FIELD_TYPE)); - root.add(new StringField("name", new BytesRef("Notebook"), Field.Store.NO)); + root.add(new StringField(rootNameField, new BytesRef("Notebook"), Field.Store.NO)); root.add(sequenceIDFields.primaryTerm); documents.add(root); iw.addDocuments(documents); @@ -603,7 +606,7 @@ public void testSubAggregationOfNested() throws Exception { assertEquals("{keyword=Stationary}", result.getBuckets().get(2).getKeyAsString()); assertEquals(1L, result.getBuckets().get(2).getDocCount()); }, - new KeywordFieldMapper.KeywordFieldType("name"), + new KeywordFieldMapper.KeywordFieldType(leafNameField), new NumberFieldMapper.NumberFieldType("price", NumberFieldMapper.NumberType.LONG) ); } From c7868a7b08a0e69559e80cb8e4730a76d15fe7a0 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Fri, 5 Feb 2021 10:28:55 -0500 Subject: [PATCH 05/12] Single nested layer tests --- .../composite/CompositeAggregatorTests.java | 77 ++++++++++++++++--- 1 file changed, 67 insertions(+), 10 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java index 39408a5bfbc05..ac0d182146194 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java @@ -564,21 +564,21 @@ public void testSubAggregationOfNested() throws Exception { useNestedDirectoryWrapping = true; SeqNoFieldMapper.SequenceIDFields sequenceIDFields = SeqNoFieldMapper.SequenceIDFields.emptySeqID(); final String leafNameField = "name"; - final String rootNameField = "product"; - // TermsValuesSourceBuilder terms = new TermsValuesSourceBuilder("keyword").field(nestedPath + ".name"); - TermsValuesSourceBuilder terms = new TermsValuesSourceBuilder("keyword").field(leafNameField); + final String rootNameField = "name"; + TermsValuesSourceBuilder terms = new TermsValuesSourceBuilder("keyword").field(nestedPath + "." + leafNameField); NestedAggregationBuilder builder = new NestedAggregationBuilder("nestedAggName", nestedPath); builder.subAggregation(new CompositeAggregationBuilder("compositeAggName", Collections.singletonList(terms))); + // Without after testCase( builder, new MatchAllDocsQuery(), iw -> { // Sub-Docs List documents = new ArrayList<>(); - documents.add(createNestedDocument("1", nestedPath, createDocument(leafNameField, "Pens and Stuff", "price" , 10L))); - documents.add(createNestedDocument("1", nestedPath, createDocument(leafNameField, "Pen World", "price" , 9L))); - documents.add(createNestedDocument("2", nestedPath, createDocument(leafNameField, "Pens and Stuff", "price" , 5L))); - documents.add(createNestedDocument("2", nestedPath, createDocument(leafNameField, "Stationary", "price" , 7L))); + documents.add(createNestedDocument("1", nestedPath, leafNameField, "Pens and Stuff", "price" , 10L)); + documents.add(createNestedDocument("1", nestedPath, leafNameField, "Pen World", "price" , 9L)); + documents.add(createNestedDocument("2", nestedPath, leafNameField, "Pens and Stuff", "price" , 5L)); + documents.add(createNestedDocument("2", nestedPath, leafNameField, "Stationary", "price" , 7L)); // Root docs Document root; root = new Document(); @@ -606,16 +606,73 @@ public void testSubAggregationOfNested() throws Exception { assertEquals("{keyword=Stationary}", result.getBuckets().get(2).getKeyAsString()); assertEquals(1L, result.getBuckets().get(2).getDocCount()); }, - new KeywordFieldMapper.KeywordFieldType(leafNameField), + new KeywordFieldMapper.KeywordFieldType(nestedPath + "." + leafNameField), new NumberFieldMapper.NumberFieldType("price", NumberFieldMapper.NumberType.LONG) ); } - private Document createNestedDocument(String id, String nestedPath, Map> fields) { + public void testSubAggregationOfNestedAggregateAfter() throws Exception { + final String nestedPath = "sellers"; + objectMappers.add(nestedObject(nestedPath)); + useNestedDirectoryWrapping = true; + SeqNoFieldMapper.SequenceIDFields sequenceIDFields = SeqNoFieldMapper.SequenceIDFields.emptySeqID(); + final String leafNameField = "name"; + final String rootNameField = "name"; + TermsValuesSourceBuilder terms = new TermsValuesSourceBuilder("keyword").field(nestedPath + "." + leafNameField); + NestedAggregationBuilder builder = new NestedAggregationBuilder("nestedAggName", nestedPath); + builder.subAggregation( + new CompositeAggregationBuilder("compositeAggName", Collections.singletonList(terms)) + .aggregateAfter(createAfterKey("keyword", "Pens and Stuff"))); + testCase( + builder, + new MatchAllDocsQuery(), + iw -> { + // Sub-Docs + List documents = new ArrayList<>(); + documents.add(createNestedDocument("1", nestedPath, leafNameField, "Pens and Stuff", "price" , 10L)); + documents.add(createNestedDocument("1", nestedPath, leafNameField, "Pen World", "price" , 9L)); + documents.add(createNestedDocument("2", nestedPath, leafNameField, "Pens and Stuff", "price" , 5L)); + documents.add(createNestedDocument("2", nestedPath, leafNameField, "Stationary", "price" , 7L)); + // Root docs + Document root; + root = new Document(); + root.add(new Field(IdFieldMapper.NAME, Uid.encodeId("1"), IdFieldMapper.Defaults.FIELD_TYPE)); + root.add(sequenceIDFields.primaryTerm); + root.add(new StringField(rootNameField, new BytesRef("Ballpoint"), Field.Store.NO)); + documents.add(root); + + root = new Document(); + root.add(new Field(IdFieldMapper.NAME, Uid.encodeId("2"), IdFieldMapper.Defaults.FIELD_TYPE)); + root.add(new StringField(rootNameField, new BytesRef("Notebook"), Field.Store.NO)); + root.add(sequenceIDFields.primaryTerm); + documents.add(root); + iw.addDocuments(documents); + }, + (InternalSingleBucketAggregation parent) -> { + assertEquals(1, parent.getAggregations().asList().size()); + InternalComposite result = (InternalComposite) parent.getProperty("compositeAggName"); + assertEquals(1, result.getBuckets().size()); + assertEquals("{keyword=Stationary}", result.afterKey().toString()); + assertEquals("{keyword=Stationary}", result.getBuckets().get(0).getKeyAsString()); + assertEquals(1L, result.getBuckets().get(0).getDocCount()); + }, + new KeywordFieldMapper.KeywordFieldType(nestedPath + "." + leafNameField), + new NumberFieldMapper.NumberFieldType("price", NumberFieldMapper.NumberType.LONG) + ); + } + + private Document createNestedDocument(String id, String nestedPath, Object... rawFields) { + assert rawFields.length % 2 == 0; Document doc = new Document(); doc.add(new Field(IdFieldMapper.NAME, Uid.encodeId(id), IdFieldMapper.Defaults.NESTED_FIELD_TYPE)); doc.add(new Field(NestedPathFieldMapper.NAME, nestedPath, NestedPathFieldMapper.Defaults.FIELD_TYPE)); - addToDocument(doc, fields); + Object[] fields = new Object[rawFields.length]; + for (int i = 0; i < fields.length; i+=2) { + assert rawFields[i] instanceof String; + fields[i] = nestedPath + "." + rawFields[i]; + fields[i+1] = rawFields[i+1]; + } + addToDocument(doc, createDocument(fields)); return doc; } From f56cb1b201180d7830869e45ffb8b2e14d550547 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Fri, 5 Feb 2021 10:31:06 -0500 Subject: [PATCH 06/12] Tests for single layer nested agg parent --- .../bucket/composite/CompositeAggregatorTests.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java index ac0d182146194..8720e1eb3980e 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java @@ -558,6 +558,9 @@ public void testUsingTestCase() throws Exception { ); } + /** + * Test using Nested aggregation as a parent of composite + */ public void testSubAggregationOfNested() throws Exception { final String nestedPath = "sellers"; objectMappers.add(nestedObject(nestedPath)); @@ -611,6 +614,9 @@ public void testSubAggregationOfNested() throws Exception { ); } + /** + * Test aggregate after with top level nested aggregation + */ public void testSubAggregationOfNestedAggregateAfter() throws Exception { final String nestedPath = "sellers"; objectMappers.add(nestedObject(nestedPath)); From e07e62761baf9bee8ab1225fdb127840294ac7df Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Fri, 5 Feb 2021 10:33:04 -0500 Subject: [PATCH 07/12] Move create nested doc method to a more sensible place --- .../composite/CompositeAggregatorTests.java | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java index 8720e1eb3980e..c857af5ffb9af 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java @@ -667,21 +667,6 @@ public void testSubAggregationOfNestedAggregateAfter() throws Exception { ); } - private Document createNestedDocument(String id, String nestedPath, Object... rawFields) { - assert rawFields.length % 2 == 0; - Document doc = new Document(); - doc.add(new Field(IdFieldMapper.NAME, Uid.encodeId(id), IdFieldMapper.Defaults.NESTED_FIELD_TYPE)); - doc.add(new Field(NestedPathFieldMapper.NAME, nestedPath, NestedPathFieldMapper.Defaults.FIELD_TYPE)); - Object[] fields = new Object[rawFields.length]; - for (int i = 0; i < fields.length; i+=2) { - assert rawFields[i] instanceof String; - fields[i] = nestedPath + "." + rawFields[i]; - fields[i+1] = rawFields[i+1]; - } - addToDocument(doc, createDocument(fields)); - return doc; - } - public void testWithKeywordAndMissingBucket() throws Exception { final List>> dataset = new ArrayList<>(); dataset.addAll( @@ -2554,6 +2539,21 @@ private static Map> createDocument(Object... fields) { return map; } + private Document createNestedDocument(String id, String nestedPath, Object... rawFields) { + assert rawFields.length % 2 == 0; + Document doc = new Document(); + doc.add(new Field(IdFieldMapper.NAME, Uid.encodeId(id), IdFieldMapper.Defaults.NESTED_FIELD_TYPE)); + doc.add(new Field(NestedPathFieldMapper.NAME, nestedPath, NestedPathFieldMapper.Defaults.FIELD_TYPE)); + Object[] fields = new Object[rawFields.length]; + for (int i = 0; i < fields.length; i+=2) { + assert rawFields[i] instanceof String; + fields[i] = nestedPath + "." + rawFields[i]; + fields[i+1] = rawFields[i+1]; + } + addToDocument(doc, createDocument(fields)); + return doc; + } + private static long asLong(String dateTime) { return DateFormatters.from(DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER.parse(dateTime)).toInstant().toEpochMilli(); } From 5d8af21d21f48fbb578bc286e6a6a5a7ede2f4ac Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Fri, 5 Feb 2021 10:56:42 -0500 Subject: [PATCH 08/12] notes on writing nested tests --- .../bucket/nested/NestedAggregatorTests.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregatorTests.java index 2eedf252cc11e..37dd411da3ed9 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregatorTests.java @@ -98,6 +98,21 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.nested; import static org.hamcrest.Matchers.equalTo; +/** + * Tests for the Nested aggregator. + * + * Notes to people wanting to add nested aggregation tests to other test classes: + *
    + *
  • Nested aggregations require a different {@link DirectoryReader} implementation than we usually use in aggregation tests. You'll + * need to override {@link AggregatorTestCase#wrapDirectoryReader} as is done in this class
  • + *
  • Nested aggregations also require object mappers to be configured. You can mock this by overriding + * {@link AggregatorTestCase#objectMappers()} as seen below
  • + *
  • In a production nested field setup, we'll automatically prefix the nested path to the leaf document field names. This helps + * prevent name collisions between "levels" of nested docs. This mechanism isn't invoked during unit tests, so preventing field name + * collisions should be done by hand. For the closest approximation of how it looks in prod, leaf docs should have field names + * prefixed with the nested path: nestedPath + "." + fieldName
  • + *
+ */ public class NestedAggregatorTests extends AggregatorTestCase { private static final String VALUE_FIELD_NAME = "number"; From 2a704d4c39287cb456829869bcbc9a26e08fe9d0 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Tue, 6 Apr 2021 18:45:21 -0400 Subject: [PATCH 09/12] Rest test for nested agg --- .../test/search.aggregation/380_nested.yml | 159 ++++++++++++++++++ 1 file changed, 159 insertions(+) create mode 100644 rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/380_nested.yml diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/380_nested.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/380_nested.yml new file mode 100644 index 0000000000000..addfded806559 --- /dev/null +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/380_nested.yml @@ -0,0 +1,159 @@ +--- +setup: + - do: + indices.create: + index: test + body: + mappings: + properties: + department: + type: keyword + staff: + type: integer + courses: + type: nested + properties: + name: + type: keyword + credits: + type: integer + sessions: + type: nested + properties: + semester: + type: keyword + + - do: + index: + index: test + id: 1 + body: { "department": "compsci", "staff": 12, "courses": [ { "name": "Object Oriented Programming", "credits": 3, "sessions": [ { "semester": "spr2021", "students": 37 }, { "semester": "fall2020", "students": 45} ] }, { "name": "Theory of Computation", "credits": 4, "sessions": [ { "semester": "spr2021", "students": 19 }, { "semester": "fall2020", "students": 14 } ] } ] } + + - do: + index: + index: test + id: 2 + body: { "department": "math", "staff": 20, "courses": [ { "name": "Precalculus", "credits": 1, "sessions": [ { "semester": "spr2021", "students": 100 }, { "semester": "fall2020", "students": 134 } ] }, { "name": "Linear Algebra", "credits": 3, "sessions": [ { "semester": "spr2021", "students": 29 }, { "semester": "fall2020", "students": 23 } ] } ] } + + +--- +"Single Level Nested with Terms": + - do: + search: + rest_total_hits_as_int: true + index: test + body: + "aggs": { + "courses": { + "nested": { "path": "courses" }, + "aggs": { + "names": { + "terms": { "field": "courses.name" } + } + } + } + } + + - match: {hits.total: 2} + - match: {aggregations.courses.doc_count: 4} + - length: { aggregations.courses.names.buckets: 4 } + - match: { aggregations.courses.names.buckets.0.key: "Linear Algebra" } + - match: { aggregations.courses.names.buckets.0.doc_count: 1} + - match: { aggregations.courses.names.buckets.1.key: "Object Oriented Programming" } + - match: { aggregations.courses.names.buckets.1.doc_count: 1} + - match: { aggregations.courses.names.buckets.2.key: "Precalculus" } + - match: { aggregations.courses.names.buckets.2.doc_count: 1} + - match: { aggregations.courses.names.buckets.3.key: "Theory of Computation" } + - match: { aggregations.courses.names.buckets.3.doc_count: 1} + +--- +"Compound nested key": + - do: + search: + rest_total_hits_as_int: true + index: test + body: + "aggs": { + "sessions": { + "nested": { "path": "courses.sessions" }, + "aggs": { + "names": { + "terms": { "field": "courses.sessions.semester" } + } + } + } + } + - match: {hits.total: 2} + - match: {aggregations.sessions.doc_count: 8} + - length: { aggregations.sessions.names.buckets: 2 } + - match: { aggregations.sessions.names.buckets.0.key: "fall2020" } + - match: { aggregations.sessions.names.buckets.0.doc_count: 4} + - match: { aggregations.sessions.names.buckets.1.key: "spr2021" } + - match: { aggregations.sessions.names.buckets.1.doc_count: 4} + +--- +"Nested with a nested sub aggregation": + - do: + search: + rest_total_hits_as_int: true + index: test + body: + "aggs": { + "courses": { + "nested": { "path": "courses" }, + "aggs": { + "sessions": { + "nested": { "path": "courses.sessions" }, + "aggs": { + "names": { + "terms": { "field": "courses.sessions.semester" } + } + } + } + } + } + } + - match: {hits.total: 2} + - match: {aggregations.courses.doc_count: 4} + - match: {aggregations.courses.sessions.doc_count: 8} + - length: { aggregations.courses.sessions.names.buckets: 2 } + - match: { aggregations.courses.sessions.names.buckets.0.key: "fall2020" } + - match: { aggregations.courses.sessions.names.buckets.0.doc_count: 4} + - match: { aggregations.courses.sessions.names.buckets.1.key: "spr2021" } + - match: { aggregations.courses.sessions.names.buckets.1.doc_count: 4} +--- +"Nested then filter then nested then terms": + - do: + search: + rest_total_hits_as_int: true + index: test + body: + "aggs": { + "courses": { + "nested": { "path": "courses" }, + "aggs": { + "highpass_filter": { + "filter": { "range": {"courses.credits": { "gt": 1 }}}, + "aggs": { + "sessions": { + "nested": { "path": "courses.sessions" }, + "aggs": { + "names": { + "terms": { "field": "courses.sessions.semester" } + } + } + } + } + } + } + } + } + - match: {hits.total: 2} + - match: {aggregations.courses.doc_count: 4} + - match: {aggregations.courses.highpass_filter.doc_count: 3} + - match: {aggregations.courses.highpass_filter.sessions.doc_count: 6} + - length: { aggregations.courses.highpass_filter.sessions.names.buckets: 2 } + - match: { aggregations.courses.highpass_filter.sessions.names.buckets.0.key: "fall2020" } + - match: { aggregations.courses.highpass_filter.sessions.names.buckets.0.doc_count: 3} + - match: { aggregations.courses.highpass_filter.sessions.names.buckets.1.key: "spr2021" } + - match: { aggregations.courses.highpass_filter.sessions.names.buckets.1.doc_count: 3} From 26a6b951da162d5cad0578fd2e0655e8278abc64 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Wed, 7 Apr 2021 10:34:56 -0400 Subject: [PATCH 10/12] oh, bother, refresh --- .../test/search.aggregation/380_nested.yml | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/380_nested.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/380_nested.yml index addfded806559..43623846c87c3 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/380_nested.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/380_nested.yml @@ -35,6 +35,9 @@ setup: id: 2 body: { "department": "math", "staff": 20, "courses": [ { "name": "Precalculus", "credits": 1, "sessions": [ { "semester": "spr2021", "students": 100 }, { "semester": "fall2020", "students": 134 } ] }, { "name": "Linear Algebra", "credits": 3, "sessions": [ { "semester": "spr2021", "students": 29 }, { "semester": "fall2020", "students": 23 } ] } ] } + - do: + indices.refresh: + index: [test] --- "Single Level Nested with Terms": @@ -43,10 +46,10 @@ setup: rest_total_hits_as_int: true index: test body: - "aggs": { + "aggregations": { "courses": { "nested": { "path": "courses" }, - "aggs": { + "aggregations": { "names": { "terms": { "field": "courses.name" } } @@ -73,10 +76,10 @@ setup: rest_total_hits_as_int: true index: test body: - "aggs": { + "aggregations": { "sessions": { "nested": { "path": "courses.sessions" }, - "aggs": { + "aggregations": { "names": { "terms": { "field": "courses.sessions.semester" } } @@ -98,13 +101,13 @@ setup: rest_total_hits_as_int: true index: test body: - "aggs": { + "aggregations": { "courses": { "nested": { "path": "courses" }, - "aggs": { + "aggregations": { "sessions": { "nested": { "path": "courses.sessions" }, - "aggs": { + "aggregations": { "names": { "terms": { "field": "courses.sessions.semester" } } @@ -128,16 +131,16 @@ setup: rest_total_hits_as_int: true index: test body: - "aggs": { + "aggregations": { "courses": { "nested": { "path": "courses" }, - "aggs": { + "aggregations": { "highpass_filter": { "filter": { "range": {"courses.credits": { "gt": 1 }}}, - "aggs": { + "aggregations": { "sessions": { "nested": { "path": "courses.sessions" }, - "aggs": { + "aggregations": { "names": { "terms": { "field": "courses.sessions.semester" } } From 8738e4d6504b741aaad2379f62a27aaf96963217 Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Wed, 7 Apr 2021 18:08:23 -0400 Subject: [PATCH 11/12] more nested testing for composite rest tests --- .../test/search.aggregation/230_composite.yml | 135 +++++++++++++++++- 1 file changed, 134 insertions(+), 1 deletion(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/230_composite.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/230_composite.yml index be3d6c60a3fee..46da3299d1541 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/230_composite.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/230_composite.yml @@ -38,6 +38,41 @@ setup: nested_long: type: long + - do: + indices.create: + index: verynested + body: + mappings: + properties: + department: + type: keyword + staff: + type: integer + courses: + type: nested + properties: + name: + type: keyword + credits: + type: integer + sessions: + type: nested + properties: + semester: + type: keyword + + - do: + index: + index: verynested + id: 1 + body: { "department": "compsci", "staff": 12, "courses": [ { "name": "Object Oriented Programming", "credits": 3, "sessions": [ { "semester": "spr2021", "students": 37 }, { "semester": "fall2020", "students": 45} ] }, { "name": "Theory of Computation", "credits": 4, "sessions": [ { "semester": "spr2021", "students": 19 }, { "semester": "fall2020", "students": 14 } ] } ] } + + - do: + index: + index: verynested + id: 2 + body: { "department": "math", "staff": 20, "courses": [ { "name": "Precalculus", "credits": 1, "sessions": [ { "semester": "spr2021", "students": 100 }, { "semester": "fall2020", "students": 134 } ] }, { "name": "Linear Algebra", "credits": 3, "sessions": [ { "semester": "spr2021", "students": 29 }, { "semester": "fall2020", "students": 23 } ] } ] } + - do: index: index: test @@ -82,7 +117,7 @@ setup: - do: indices.refresh: - index: [test, other] + index: [test, other, verynested] --- "Simple Composite aggregation": @@ -987,3 +1022,101 @@ setup: - length: { aggregations.test.buckets: 1 } - match: { aggregations.test.buckets.0.key.keyword: "foo" } - match: { aggregations.test.buckets.0.doc_count: 1 } + +--- +"Nested as parent": + - do: + search: + rest_total_hits_as_int: true + index: verynested + body: + "aggregations": { + "courses": { + "nested": { "path": "courses" }, + "aggregations": { + "names": { + "composite": { + "sources": [ + "kw": {"terms": {"field": "courses.name"}} + ] + } + } + } + } + } + + - match: {hits.total: 2} + - match: {aggregations.courses.doc_count: 4} + - length: { aggregations.courses.names.buckets: 4 } + - match: { aggregations.courses.names.buckets.0.key.kw: "Linear Algebra" } + - match: { aggregations.courses.names.buckets.0.doc_count: 1} + - match: { aggregations.courses.names.buckets.1.key.kw: "Object Oriented Programming" } + - match: { aggregations.courses.names.buckets.1.doc_count: 1} + - match: { aggregations.courses.names.buckets.2.key.kw: "Precalculus" } + - match: { aggregations.courses.names.buckets.2.doc_count: 1} + - match: { aggregations.courses.names.buckets.3.key.kw: "Theory of Computation" } + - match: { aggregations.courses.names.buckets.3.doc_count: 1} + +--- +"Nested parent with compound key": + - do: + search: + rest_total_hits_as_int: true + index: verynested + body: + "aggregations": { + "sessions": { + "nested": { "path": "courses.sessions" }, + "aggregations": { + "names": { + "composite": { + "sources": [ + "kw": { "terms": { "field": "courses.sessions.semester" } } + ] + } + } + } + } + } + - match: {hits.total: 2} + - match: {aggregations.sessions.doc_count: 8} + - length: { aggregations.sessions.names.buckets: 2 } + - match: { aggregations.sessions.names.buckets.0.key.kw: "fall2020" } + - match: { aggregations.sessions.names.buckets.0.doc_count: 4} + - match: { aggregations.sessions.names.buckets.1.key.kw: "spr2021" } + - match: { aggregations.sessions.names.buckets.1.doc_count: 4} + +--- +"Nested with a nested sub aggregation": + - do: + search: + rest_total_hits_as_int: true + index: verynested + body: + "aggregations": { + "courses": { + "nested": { "path": "courses" }, + "aggregations": { + "sessions": { + "nested": { "path": "courses.sessions" }, + "aggregations": { + "names": { + "composite": { + "sources": [ + "kw": {"terms": { "field": "courses.sessions.semester" }} + ] + } + } + } + } + } + } + } + - match: {hits.total: 2} + - match: {aggregations.courses.doc_count: 4} + - match: {aggregations.courses.sessions.doc_count: 8} + - length: { aggregations.courses.sessions.names.buckets: 2 } + - match: { aggregations.courses.sessions.names.buckets.0.key.kw: "fall2020" } + - match: { aggregations.courses.sessions.names.buckets.0.doc_count: 4} + - match: { aggregations.courses.sessions.names.buckets.1.key.kw: "spr2021" } + - match: { aggregations.courses.sessions.names.buckets.1.doc_count: 4} From 1a0c19429d53e4a01324048879018261bd28ea3a Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Thu, 8 Apr 2021 10:47:09 -0400 Subject: [PATCH 12/12] response to PR feedback --- .../bucket/composite/CompositeAggregatorTests.java | 6 +----- .../aggregations/bucket/nested/NestedAggregatorTests.java | 1 + 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java index 85fa0677202d5..ceeea44fd56ca 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java @@ -101,7 +101,6 @@ public class CompositeAggregatorTests extends AggregatorTestCase { private static MappedFieldType[] FIELD_TYPES; private List objectMappers; - private boolean useNestedDirectoryWrapping = false; @Override @Before @@ -126,7 +125,6 @@ public void tearDown() throws Exception { super.tearDown(); FIELD_TYPES = null; objectMappers = null; - useNestedDirectoryWrapping = false; } @Override @@ -136,7 +134,7 @@ protected List objectMappers() { @Override protected IndexReader wrapDirectoryReader(DirectoryReader reader) throws IOException { - if (useNestedDirectoryWrapping) { + if (false == objectMappers().isEmpty()) { return wrapInMockESDirectoryReader(reader); } return reader; @@ -649,7 +647,6 @@ public void testUsingTestCase() throws Exception { public void testSubAggregationOfNested() throws Exception { final String nestedPath = "sellers"; objectMappers.add(nestedObject(nestedPath)); - useNestedDirectoryWrapping = true; SeqNoFieldMapper.SequenceIDFields sequenceIDFields = SeqNoFieldMapper.SequenceIDFields.emptySeqID(); final String leafNameField = "name"; final String rootNameField = "name"; @@ -705,7 +702,6 @@ public void testSubAggregationOfNested() throws Exception { public void testSubAggregationOfNestedAggregateAfter() throws Exception { final String nestedPath = "sellers"; objectMappers.add(nestedObject(nestedPath)); - useNestedDirectoryWrapping = true; SeqNoFieldMapper.SequenceIDFields sequenceIDFields = SeqNoFieldMapper.SequenceIDFields.emptySeqID(); final String leafNameField = "name"; final String rootNameField = "name"; diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregatorTests.java index 27f2682b7eaee..c3a64d780c9fe 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregatorTests.java @@ -90,6 +90,7 @@ /** * Tests for the Nested aggregator. * + *

* Notes to people wanting to add nested aggregation tests to other test classes: *

    *
  • Nested aggregations require a different {@link DirectoryReader} implementation than we usually use in aggregation tests. You'll