From 9dcc56f08583637f474e2cbfd833b8b4413e35f2 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Tue, 27 Mar 2018 12:59:14 +0200 Subject: [PATCH 1/2] Propagate ignore_unmapped to inner_hits In 5.2 `ignore_unmapped` was added to `inner_hits` in order to ignore invalid mapping. This value was automatically set to the value defined in the parent query (`nested`, `has_child`, `has_parent`) but the refactoring of the parent/child in 5.6 removed this behavior unintentionally. This commit restores this behavior but also makes sure that we always automatically enforce this value when the query builder is used directly (previously this was only done by the XContent deserialization). Closes #29071 --- .../elasticsearch/join/query/HasChildQueryBuilder.java | 5 ++++- .../elasticsearch/join/query/HasParentQueryBuilder.java | 4 ++++ .../join/query/HasChildQueryBuilderTests.java | 9 +++++++-- .../join/query/HasParentQueryBuilderTests.java | 6 ++++++ .../elasticsearch/index/query/NestedQueryBuilder.java | 4 ++++ .../index/query/NestedQueryBuilderTests.java | 3 ++- 6 files changed, 27 insertions(+), 4 deletions(-) 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 65a02b9c83e5d..0577aa01ebd8f 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 @@ -167,6 +167,7 @@ public InnerHitBuilder innerHit() { public HasChildQueryBuilder innerHit(InnerHitBuilder innerHit) { this.innerHitBuilder = innerHit; + innerHitBuilder.setIgnoreUnmapped(ignoreUnmapped); return this; } @@ -212,6 +213,9 @@ public int minChildren() { */ public HasChildQueryBuilder ignoreUnmapped(boolean ignoreUnmapped) { this.ignoreUnmapped = ignoreUnmapped; + if (innerHitBuilder!= null ){ + innerHitBuilder.setIgnoreUnmapped(ignoreUnmapped); + } return this; } @@ -291,7 +295,6 @@ public static HasChildQueryBuilder fromXContent(XContentParser parser) throws IO hasChildQueryBuilder.ignoreUnmapped(ignoreUnmapped); if (innerHitBuilder != null) { hasChildQueryBuilder.innerHit(innerHitBuilder); - hasChildQueryBuilder.ignoreUnmapped(ignoreUnmapped); } return hasChildQueryBuilder; } 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 cce6cdc840479..5e2dd4206f2f7 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 @@ -145,6 +145,7 @@ public InnerHitBuilder innerHit() { public HasParentQueryBuilder innerHit(InnerHitBuilder innerHit) { this.innerHitBuilder = innerHit; + innerHitBuilder.setIgnoreUnmapped(ignoreUnmapped); return this; } @@ -155,6 +156,9 @@ public HasParentQueryBuilder innerHit(InnerHitBuilder innerHit) { */ public HasParentQueryBuilder ignoreUnmapped(boolean ignoreUnmapped) { this.ignoreUnmapped = ignoreUnmapped; + if (innerHitBuilder != null) { + innerHitBuilder.setIgnoreUnmapped(ignoreUnmapped); + } return this; } 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 0dcf5933f4f23..f7aa32a822c9b 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 @@ -158,8 +158,7 @@ protected HasChildQueryBuilder doCreateTestQueryBuilder() { hqb.innerHit(new InnerHitBuilder() .setName(randomAlphaOfLengthBetween(1, 10)) .setSize(randomIntBetween(0, 100)) - .addSort(new FieldSortBuilder(STRING_FIELD_NAME_2).order(SortOrder.ASC)) - .setIgnoreUnmapped(hqb.ignoreUnmapped())); + .addSort(new FieldSortBuilder(STRING_FIELD_NAME_2).order(SortOrder.ASC))); } return hqb; } @@ -345,13 +344,19 @@ public void testNonDefaultSimilarity() throws Exception { public void testIgnoreUnmapped() throws IOException { final HasChildQueryBuilder queryBuilder = new HasChildQueryBuilder("unmapped", new MatchAllQueryBuilder(), ScoreMode.None); + queryBuilder.innerHit(new InnerHitBuilder()); + assertFalse(queryBuilder.innerHit().isIgnoreUnmapped()); queryBuilder.ignoreUnmapped(true); + assertTrue(queryBuilder.innerHit().isIgnoreUnmapped()); Query query = queryBuilder.toQuery(createShardContext()); assertThat(query, notNullValue()); assertThat(query, instanceOf(MatchNoDocsQuery.class)); final HasChildQueryBuilder failingQueryBuilder = new HasChildQueryBuilder("unmapped", new MatchAllQueryBuilder(), ScoreMode.None); + queryBuilder.innerHit(new InnerHitBuilder()); + assertFalse(queryBuilder.innerHit().isIgnoreUnmapped()); failingQueryBuilder.ignoreUnmapped(false); + assertFalse(queryBuilder.innerHit().isIgnoreUnmapped()); QueryShardException e = expectThrows(QueryShardException.class, () -> failingQueryBuilder.toQuery(createShardContext())); assertThat(e.getMessage(), containsString("[" + HasChildQueryBuilder.NAME + "] join field [join_field] doesn't hold [unmapped] as a child")); 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 c7ded186c9aee..1bb391105c7f2 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 @@ -245,13 +245,19 @@ public void testFromJson() throws IOException { public void testIgnoreUnmapped() throws IOException { final HasParentQueryBuilder queryBuilder = new HasParentQueryBuilder("unmapped", new MatchAllQueryBuilder(), false); + queryBuilder.innerHit(new InnerHitBuilder()); + assertFalse(queryBuilder.innerHit().isIgnoreUnmapped()); queryBuilder.ignoreUnmapped(true); + assertTrue(queryBuilder.innerHit().isIgnoreUnmapped()); Query query = queryBuilder.toQuery(createShardContext()); assertThat(query, notNullValue()); assertThat(query, instanceOf(MatchNoDocsQuery.class)); final HasParentQueryBuilder failingQueryBuilder = new HasParentQueryBuilder("unmapped", new MatchAllQueryBuilder(), false); + queryBuilder.innerHit(new InnerHitBuilder()); + assertFalse(queryBuilder.innerHit().isIgnoreUnmapped()); failingQueryBuilder.ignoreUnmapped(false); + assertFalse(queryBuilder.innerHit().isIgnoreUnmapped()); QueryShardException e = expectThrows(QueryShardException.class, () -> failingQueryBuilder.toQuery(createShardContext())); assertThat(e.getMessage(), containsString("[has_parent] join field [join_field] doesn't hold [unmapped] as a parent")); 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 9ebd548cae1f0..889f41a037f86 100644 --- a/server/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/NestedQueryBuilder.java @@ -132,6 +132,7 @@ public InnerHitBuilder innerHit() { public NestedQueryBuilder innerHit(InnerHitBuilder innerHitBuilder) { this.innerHitBuilder = innerHitBuilder; + innerHitBuilder.setIgnoreUnmapped(ignoreUnmapped); return this; } @@ -149,6 +150,9 @@ public ScoreMode scoreMode() { */ public NestedQueryBuilder ignoreUnmapped(boolean ignoreUnmapped) { this.ignoreUnmapped = ignoreUnmapped; + if (innerHitBuilder != null) { + innerHitBuilder.setIgnoreUnmapped(ignoreUnmapped); + } return this; } 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 46e10bc7f224c..a2e6018d0ef6b 100644 --- a/server/src/test/java/org/elasticsearch/index/query/NestedQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/NestedQueryBuilderTests.java @@ -347,7 +347,8 @@ public void testBuildIgnoreUnmappedNestQuery() throws Exception { }); innerHitBuilders.clear(); NestedQueryBuilder query2 = new NestedQueryBuilder("path", new MatchAllQueryBuilder(), ScoreMode.None); - query2.innerHit(leafInnerHits.setIgnoreUnmapped(true)); + query2.ignoreUnmapped(true); + query2.innerHit(leafInnerHits); query2.extractInnerHitBuilders(innerHitBuilders); assertThat(innerHitBuilders.size(), Matchers.equalTo(1)); assertTrue(innerHitBuilders.containsKey(leafInnerHits.getName())); From c996bcaa993fe3da5eecb8c167405e6c274a96b5 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Tue, 27 Mar 2018 16:03:59 +0200 Subject: [PATCH 2/2] fix ut --- .../join/query/HasChildQueryBuilderTests.java | 6 +++--- .../join/query/HasParentQueryBuilderTests.java | 9 ++++----- 2 files changed, 7 insertions(+), 8 deletions(-) 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 f7aa32a822c9b..4f4d965d59433 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 @@ -353,10 +353,10 @@ public void testIgnoreUnmapped() throws IOException { assertThat(query, instanceOf(MatchNoDocsQuery.class)); final HasChildQueryBuilder failingQueryBuilder = new HasChildQueryBuilder("unmapped", new MatchAllQueryBuilder(), ScoreMode.None); - queryBuilder.innerHit(new InnerHitBuilder()); - assertFalse(queryBuilder.innerHit().isIgnoreUnmapped()); + failingQueryBuilder.innerHit(new InnerHitBuilder()); + assertFalse(failingQueryBuilder.innerHit().isIgnoreUnmapped()); failingQueryBuilder.ignoreUnmapped(false); - assertFalse(queryBuilder.innerHit().isIgnoreUnmapped()); + assertFalse(failingQueryBuilder.innerHit().isIgnoreUnmapped()); QueryShardException e = expectThrows(QueryShardException.class, () -> failingQueryBuilder.toQuery(createShardContext())); assertThat(e.getMessage(), containsString("[" + HasChildQueryBuilder.NAME + "] join field [join_field] doesn't hold [unmapped] as a child")); 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 1bb391105c7f2..e2d45d22ab25d 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 @@ -132,8 +132,7 @@ protected HasParentQueryBuilder doCreateTestQueryBuilder() { hqb.innerHit(new InnerHitBuilder() .setName(randomAlphaOfLengthBetween(1, 10)) .setSize(randomIntBetween(0, 100)) - .addSort(new FieldSortBuilder(STRING_FIELD_NAME_2).order(SortOrder.ASC)) - .setIgnoreUnmapped(hqb.ignoreUnmapped())); + .addSort(new FieldSortBuilder(STRING_FIELD_NAME_2).order(SortOrder.ASC))); } return hqb; } @@ -254,10 +253,10 @@ public void testIgnoreUnmapped() throws IOException { assertThat(query, instanceOf(MatchNoDocsQuery.class)); final HasParentQueryBuilder failingQueryBuilder = new HasParentQueryBuilder("unmapped", new MatchAllQueryBuilder(), false); - queryBuilder.innerHit(new InnerHitBuilder()); - assertFalse(queryBuilder.innerHit().isIgnoreUnmapped()); + failingQueryBuilder.innerHit(new InnerHitBuilder()); + assertFalse(failingQueryBuilder.innerHit().isIgnoreUnmapped()); failingQueryBuilder.ignoreUnmapped(false); - assertFalse(queryBuilder.innerHit().isIgnoreUnmapped()); + assertFalse(failingQueryBuilder.innerHit().isIgnoreUnmapped()); QueryShardException e = expectThrows(QueryShardException.class, () -> failingQueryBuilder.toQuery(createShardContext())); assertThat(e.getMessage(), containsString("[has_parent] join field [join_field] doesn't hold [unmapped] as a parent"));