Merge pull request #17167 from martijnvg/parent_id_query_take_child_type_into_account

parent_id query should take the child type into account too

core/src/main/java/org/elasticsearch/index/query/ParentIdQueryBuilder.java

@@ -19,13 +19,18 @@
 
 package org.elasticsearch.index.query;
 
+import org.apache.lucene.index.Term;
+import org.apache.lucene.search.BooleanClause;
+import org.apache.lucene.search.BooleanQuery;
 import org.apache.lucene.search.DocValuesTermsQuery;
 import org.apache.lucene.search.Query;
+import org.apache.lucene.search.TermQuery;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.index.mapper.DocumentMapper;
 import org.elasticsearch.index.mapper.internal.ParentFieldMapper;
+import org.elasticsearch.index.mapper.internal.TypeFieldMapper;
 
 import java.io.IOException;
 import java.util.Objects;
@@ -71,7 +76,12 @@ public final class ParentIdQueryBuilder extends AbstractQueryBuilder<ParentIdQue
             throw new QueryShardException(context, "[" + NAME + "] _parent field has no parent type configured");
         }
         String fieldName = ParentFieldMapper.joinField(parentFieldMapper.type());
-        return new DocValuesTermsQuery(fieldName, id);
+
+        BooleanQuery.Builder query = new BooleanQuery.Builder();
+        query.add(new DocValuesTermsQuery(fieldName, id), BooleanClause.Occur.MUST);
+        // Need to take child type into account, otherwise a child doc of different type with the same id could match
+        query.add(new TermQuery(new Term(TypeFieldMapper.NAME, type)), BooleanClause.Occur.FILTER);
+        return query.build();
     }
 
     @Override

core/src/test/java/org/elasticsearch/index/query/ParentIdQueryBuilderTests.java

@@ -19,12 +19,15 @@
 
 package org.elasticsearch.index.query;
 
+import org.apache.lucene.search.BooleanQuery;
 import org.apache.lucene.search.DocValuesTermsQuery;
 import org.apache.lucene.search.Query;
+import org.apache.lucene.search.TermQuery;
 import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
 import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.index.fielddata.IndexFieldDataService;
 import org.elasticsearch.index.mapper.MapperService;
+import org.elasticsearch.index.mapper.internal.TypeFieldMapper;
 import org.elasticsearch.search.fetch.innerhits.InnerHitsContext;
 import org.elasticsearch.search.internal.SearchContext;
 import org.elasticsearch.test.TestSearchContext;
@@ -87,11 +90,17 @@ public class ParentIdQueryBuilderTests extends AbstractQueryTestCase<ParentIdQue
 
     @Override
     protected void doAssertLuceneQuery(ParentIdQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
-        assertThat(query, Matchers.instanceOf(DocValuesTermsQuery.class));
-        DocValuesTermsQuery termsQuery = (DocValuesTermsQuery) query;
+        assertThat(query, Matchers.instanceOf(BooleanQuery.class));
+        BooleanQuery booleanQuery = (BooleanQuery) query;
+        assertThat(booleanQuery.clauses().size(), Matchers.equalTo(2));
+        DocValuesTermsQuery idQuery = (DocValuesTermsQuery) booleanQuery.clauses().get(0).getQuery();
         // there are no getters to get the field and terms on DocValuesTermsQuery, so lets validate by creating a
         // new query based on the builder:
-        assertThat(termsQuery, Matchers.equalTo(new DocValuesTermsQuery("_parent#" + PARENT_TYPE, queryBuilder.getId())));
+        assertThat(idQuery, Matchers.equalTo(new DocValuesTermsQuery("_parent#" + PARENT_TYPE, queryBuilder.getId())));
+
+        TermQuery typeQuery = (TermQuery) booleanQuery.clauses().get(1).getQuery();
+        assertThat(typeQuery.getTerm().field(), Matchers.equalTo(TypeFieldMapper.NAME));
+        assertThat(typeQuery.getTerm().text(), Matchers.equalTo(queryBuilder.getType()));
     }
 
     public void testFromJson() throws IOException {