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.
This commit is contained in:
Martijn van Groningen 2016-03-17 14:58:57 +01:00
parent 6441852618
commit 68282dd9e9
2 changed files with 23 additions and 4 deletions

View File

@ -19,13 +19,18 @@
package org.elasticsearch.index.query; 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.DocValuesTermsQuery;
import org.apache.lucene.search.Query; 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.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.internal.ParentFieldMapper; import org.elasticsearch.index.mapper.internal.ParentFieldMapper;
import org.elasticsearch.index.mapper.internal.TypeFieldMapper;
import java.io.IOException; import java.io.IOException;
import java.util.Objects; 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"); throw new QueryShardException(context, "[" + NAME + "] _parent field has no parent type configured");
} }
String fieldName = ParentFieldMapper.joinField(parentFieldMapper.type()); 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 @Override

View File

@ -19,12 +19,15 @@
package org.elasticsearch.index.query; package org.elasticsearch.index.query;
import org.apache.lucene.search.BooleanQuery;
import org.apache.lucene.search.DocValuesTermsQuery; import org.apache.lucene.search.DocValuesTermsQuery;
import org.apache.lucene.search.Query; import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermQuery;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.index.fielddata.IndexFieldDataService; import org.elasticsearch.index.fielddata.IndexFieldDataService;
import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.internal.TypeFieldMapper;
import org.elasticsearch.search.fetch.innerhits.InnerHitsContext; import org.elasticsearch.search.fetch.innerhits.InnerHitsContext;
import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.search.internal.SearchContext;
import org.elasticsearch.test.TestSearchContext; import org.elasticsearch.test.TestSearchContext;
@ -87,11 +90,17 @@ public class ParentIdQueryBuilderTests extends AbstractQueryTestCase<ParentIdQue
@Override @Override
protected void doAssertLuceneQuery(ParentIdQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException { protected void doAssertLuceneQuery(ParentIdQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
assertThat(query, Matchers.instanceOf(DocValuesTermsQuery.class)); assertThat(query, Matchers.instanceOf(BooleanQuery.class));
DocValuesTermsQuery termsQuery = (DocValuesTermsQuery) query; 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 // there are no getters to get the field and terms on DocValuesTermsQuery, so lets validate by creating a
// new query based on the builder: // 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 { public void testFromJson() throws IOException {