From 68282dd9e9833fa16ce413f7bb46ea808ddb5598 Mon Sep 17 00:00:00 2001
From: Martijn van Groningen <martijn.v.groningen@gmail.com>
Date: Thu, 17 Mar 2016 14:58:57 +0100
Subject: [PATCH] parent/child: `parent_id` query should take the child type
 into account too.

If this query doesn't take the child type into account then it can match other
child document types pointing to the same parent type and that have the same id too.
---
 .../index/query/ParentIdQueryBuilder.java         | 12 +++++++++++-
 .../index/query/ParentIdQueryBuilderTests.java    | 15 ++++++++++++---
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/core/src/main/java/org/elasticsearch/index/query/ParentIdQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/ParentIdQueryBuilder.java
index f9bd7623f35..4a2efa95c9a 100644
--- a/core/src/main/java/org/elasticsearch/index/query/ParentIdQueryBuilder.java
+++ b/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
diff --git a/core/src/test/java/org/elasticsearch/index/query/ParentIdQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/ParentIdQueryBuilderTests.java
index 8ae2145533e..aa38716d4e6 100644
--- a/core/src/test/java/org/elasticsearch/index/query/ParentIdQueryBuilderTests.java
+++ b/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 {