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 696c4a72bdba8..1c44daea4e982 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 @@ -460,9 +460,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 e98fdb9e9699d..30a2718aab054 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 @@ -285,9 +285,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 eea01d61386de..2a28e232b5eda 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 @@ -367,4 +367,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 164405f653444..ea77ad80799ba 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 @@ -268,4 +268,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 3c3856e208f04..ee8062308ac11 100644 --- a/server/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java @@ -317,10 +317,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 ac9ae8d0fa7fb..a3b6376a048f2 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; @@ -354,4 +355,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 d68c85ab652ae..14fa6a9f565ef 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; @@ -674,4 +679,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]")); + } }