diff --git a/modules/parent-join/src/main/java/org/elasticsearch/join/query/HasChildQueryBuilder.java b/modules/parent-join/src/main/java/org/elasticsearch/join/query/HasChildQueryBuilder.java index b79df73a9189f..9424fadafb572 100644 --- a/modules/parent-join/src/main/java/org/elasticsearch/join/query/HasChildQueryBuilder.java +++ b/modules/parent-join/src/main/java/org/elasticsearch/join/query/HasChildQueryBuilder.java @@ -517,9 +517,13 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryShardContext) throws I @Override protected void extractInnerHitBuilders(Map innerHits) { if (innerHitBuilder != null) { + String name = innerHitBuilder.getName() != null ? innerHitBuilder.getName() : type; + if (innerHits.containsKey(name)) { + throw new IllegalArgumentException("[inner_hits] already contains an entry for key [" + name + "]"); + } + Map children = new HashMap<>(); InnerHitContextBuilder.extractInnerHits(query, children); - String name = innerHitBuilder.getName() != null ? innerHitBuilder.getName() : type; InnerHitContextBuilder innerHitContextBuilder = new ParentChildInnerHitContextBuilder(type, true, query, innerHitBuilder, children); innerHits.put(name, innerHitContextBuilder); diff --git a/modules/parent-join/src/main/java/org/elasticsearch/join/query/HasParentQueryBuilder.java b/modules/parent-join/src/main/java/org/elasticsearch/join/query/HasParentQueryBuilder.java index 9d979d149d502..ab74dc2b14c58 100644 --- a/modules/parent-join/src/main/java/org/elasticsearch/join/query/HasParentQueryBuilder.java +++ b/modules/parent-join/src/main/java/org/elasticsearch/join/query/HasParentQueryBuilder.java @@ -367,9 +367,13 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryShardContext) throws I @Override protected void extractInnerHitBuilders(Map innerHits) { if (innerHitBuilder != null) { + String name = innerHitBuilder.getName() != null ? innerHitBuilder.getName() : type; + if (innerHits.containsKey(name)) { + throw new IllegalArgumentException("[inner_hits] already contains an entry for key [" + name + "]"); + } + Map children = new HashMap<>(); InnerHitContextBuilder.extractInnerHits(query, children); - String name = innerHitBuilder.getName() != null ? innerHitBuilder.getName() : type; InnerHitContextBuilder innerHitContextBuilder = new ParentChildInnerHitContextBuilder(type, false, query, innerHitBuilder, children); innerHits.put(name, innerHitContextBuilder); diff --git a/modules/parent-join/src/test/java/org/elasticsearch/join/query/HasChildQueryBuilderTests.java b/modules/parent-join/src/test/java/org/elasticsearch/join/query/HasChildQueryBuilderTests.java index f26a98482294d..cbba36b742ffb 100644 --- a/modules/parent-join/src/test/java/org/elasticsearch/join/query/HasChildQueryBuilderTests.java +++ b/modules/parent-join/src/test/java/org/elasticsearch/join/query/HasChildQueryBuilderTests.java @@ -371,4 +371,12 @@ public void testIgnoreUnmappedWithRewrite() throws IOException { assertThat(query, notNullValue()); assertThat(query, instanceOf(MatchNoDocsQuery.class)); } + + public void testExtractInnerHitBuildersWithDuplicate() { + final HasChildQueryBuilder queryBuilder + = new HasChildQueryBuilder(CHILD_DOC, new WrapperQueryBuilder(new MatchAllQueryBuilder().toString()), ScoreMode.None); + queryBuilder.innerHit(new InnerHitBuilder("some_name")); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> InnerHitContextBuilder.extractInnerHits(queryBuilder, Collections.singletonMap("some_name", null))); + } } diff --git a/modules/parent-join/src/test/java/org/elasticsearch/join/query/HasParentQueryBuilderTests.java b/modules/parent-join/src/test/java/org/elasticsearch/join/query/HasParentQueryBuilderTests.java index 3a66ea380a526..afacedda944cc 100644 --- a/modules/parent-join/src/test/java/org/elasticsearch/join/query/HasParentQueryBuilderTests.java +++ b/modules/parent-join/src/test/java/org/elasticsearch/join/query/HasParentQueryBuilderTests.java @@ -272,4 +272,12 @@ public void testIgnoreUnmappedWithRewrite() throws IOException { assertThat(query, notNullValue()); assertThat(query, instanceOf(MatchNoDocsQuery.class)); } + + public void testExtractInnerHitBuildersWithDuplicate() { + final HasParentQueryBuilder queryBuilder + = new HasParentQueryBuilder(CHILD_DOC, new WrapperQueryBuilder(new MatchAllQueryBuilder().toString()), false); + queryBuilder.innerHit(new InnerHitBuilder("some_name")); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> InnerHitContextBuilder.extractInnerHits(queryBuilder, Collections.singletonMap("some_name", null))); + } } diff --git a/server/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java index f1895f524f5a6..4a42a72c64baf 100644 --- a/server/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java @@ -332,10 +332,14 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws @Override public void extractInnerHitBuilders(Map innerHits) { if (innerHitBuilder != null) { + String name = innerHitBuilder.getName() != null ? innerHitBuilder.getName() : path; + if (innerHits.containsKey(name)) { + throw new IllegalArgumentException("[inner_hits] already contains an entry for key [" + name + "]"); + } + Map children = new HashMap<>(); InnerHitContextBuilder.extractInnerHits(query, children); InnerHitContextBuilder innerHitContextBuilder = new NestedInnerHitContextBuilder(path, query, innerHitBuilder, children); - String name = innerHitBuilder.getName() != null ? innerHitBuilder.getName() : path; innerHits.put(name, innerHitContextBuilder); } } diff --git a/server/src/test/java/org/elasticsearch/index/query/NestedQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/NestedQueryBuilderTests.java index 409151e6a19d6..f922729ef2c28 100644 --- a/server/src/test/java/org/elasticsearch/index/query/NestedQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/NestedQueryBuilderTests.java @@ -41,6 +41,7 @@ import org.hamcrest.Matchers; import java.io.IOException; +import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -358,4 +359,12 @@ public void testBuildIgnoreUnmappedNestQuery() throws Exception { nestedContextBuilder.build(searchContext, innerHitsContext); assertThat(innerHitsContext.getInnerHits().size(), Matchers.equalTo(0)); } + + public void testExtractInnerHitBuildersWithDuplicate() { + final NestedQueryBuilder queryBuilder + = new NestedQueryBuilder("path", new WrapperQueryBuilder(new MatchAllQueryBuilder().toString()), ScoreMode.None); + queryBuilder.innerHit(new InnerHitBuilder("some_name")); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> InnerHitContextBuilder.extractInnerHits(queryBuilder,Collections.singletonMap("some_name", null))); + } } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/NestedIT.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/NestedIT.java index d5f93f0daa704..e78afc1c336c2 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/NestedIT.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/NestedIT.java @@ -21,10 +21,13 @@ import org.apache.lucene.search.join.ScoreMode; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.search.SearchPhaseExecutionException; +import org.elasticsearch.action.search.SearchRequestBuilder; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.index.query.InnerHitBuilder; +import org.elasticsearch.rest.RestStatus; import org.elasticsearch.search.aggregations.Aggregator.SubAggCollectionMode; import org.elasticsearch.search.aggregations.InternalAggregation; import org.elasticsearch.search.aggregations.bucket.filter.Filter; @@ -46,6 +49,7 @@ import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_REPLICAS; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; +import static org.elasticsearch.index.query.QueryBuilders.boolQuery; import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; import static org.elasticsearch.index.query.QueryBuilders.nestedQuery; import static org.elasticsearch.index.query.QueryBuilders.termQuery; @@ -57,6 +61,7 @@ import static org.elasticsearch.search.aggregations.AggregationBuilders.sum; import static org.elasticsearch.search.aggregations.AggregationBuilders.terms; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertFailures; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoFailures; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse; @@ -667,4 +672,46 @@ public void testFilterAggInsideNestedAgg() throws Exception { numStringParams = bucket.getAggregations().get("num_string_params"); assertThat(numStringParams.getDocCount(), equalTo(0L)); } + + public void testExtractInnerHitBuildersWithDuplicateHitName() throws Exception { + assertAcked( + prepareCreate("idxduplicatehitnames") + .setSettings(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 1).put(SETTING_NUMBER_OF_REPLICAS, 0)) + .addMapping("product", "categories", "type=keyword", "name", "type=text", "property", "type=nested") + ); + ensureGreen("idxduplicatehitnames"); + + SearchRequestBuilder searchRequestBuilder = client() + .prepareSearch("idxduplicatehitnames") + .setQuery(boolQuery() + .should(nestedQuery("property", termQuery("property.id", 1D), ScoreMode.None).innerHit(new InnerHitBuilder("ih1"))) + .should(nestedQuery("property", termQuery("property.id", 1D), ScoreMode.None).innerHit(new InnerHitBuilder("ih2"))) + .should(nestedQuery("property", termQuery("property.id", 1D), ScoreMode.None).innerHit(new InnerHitBuilder("ih1")))); + + assertFailures( + searchRequestBuilder, + RestStatus.BAD_REQUEST, + containsString("[inner_hits] already contains an entry for key [ih1]")); + } + + public void testExtractInnerHitBuildersWithDuplicatePath() throws Exception { + assertAcked( + prepareCreate("idxnullhitnames") + .setSettings(Settings.builder().put(SETTING_NUMBER_OF_SHARDS, 1).put(SETTING_NUMBER_OF_REPLICAS, 0)) + .addMapping("product", "categories", "type=keyword", "name", "type=text", "property", "type=nested") + ); + ensureGreen("idxnullhitnames"); + + SearchRequestBuilder searchRequestBuilder = client() + .prepareSearch("idxnullhitnames") + .setQuery(boolQuery() + .should(nestedQuery("property", termQuery("property.id", 1D), ScoreMode.None).innerHit(new InnerHitBuilder())) + .should(nestedQuery("property", termQuery("property.id", 1D), ScoreMode.None).innerHit(new InnerHitBuilder())) + .should(nestedQuery("property", termQuery("property.id", 1D), ScoreMode.None).innerHit(new InnerHitBuilder()))); + + assertFailures( + searchRequestBuilder, + RestStatus.BAD_REQUEST, + containsString("[inner_hits] already contains an entry for key [property]")); + } }