Browse Source

Fixed incorrect results with `has_child` query with score mode if the parent type has nested object types. The inner objects (Separate Lucene docs) are also emitted as hits, which incorrectly decreased the count down short circuit mechanism in the `has_child` query.

Closes #4341
Martijn van Groningen 12 years ago
parent
commit
dd86db3347

+ 7 - 1
src/main/java/org/elasticsearch/index/query/HasChildFilterParser.java

@@ -30,6 +30,7 @@ import org.elasticsearch.index.mapper.DocumentMapper;
 import org.elasticsearch.index.search.child.ChildrenConstantScoreQuery;
 import org.elasticsearch.index.search.child.CustomQueryWrappingFilter;
 import org.elasticsearch.index.search.child.DeleteByQueryWrappingFilter;
+import org.elasticsearch.index.search.nested.NonNestedDocsFilter;
 import org.elasticsearch.search.internal.SearchContext;
 
 import java.io.IOException;
@@ -134,8 +135,13 @@ public class HasChildFilterParser implements FilterParser {
             throw new QueryParsingException(parseContext.index(), "[has_child]  Type [" + childType + "] points to a non existent parent type [" + parentType + "]");
         }
 
+        Filter nonNestedDocsFilter = null;
+        if (parentDocMapper.hasNestedObjects()) {
+            nonNestedDocsFilter = parseContext.cacheFilter(NonNestedDocsFilter.INSTANCE, null);
+        }
+
         Filter parentFilter = parseContext.cacheFilter(parentDocMapper.typeFilter(), null);
-        Query childrenConstantScoreQuery = new ChildrenConstantScoreQuery(query, parentType, childType, parentFilter, shortCircuitParentDocSet);
+        Query childrenConstantScoreQuery = new ChildrenConstantScoreQuery(query, parentType, childType, parentFilter, shortCircuitParentDocSet, nonNestedDocsFilter);
 
         if (filterName != null) {
             parseContext.addNamedQuery(filterName, childrenConstantScoreQuery);

+ 8 - 2
src/main/java/org/elasticsearch/index/query/HasChildQueryParser.java

@@ -31,6 +31,7 @@ import org.elasticsearch.index.search.child.ChildrenConstantScoreQuery;
 import org.elasticsearch.index.search.child.ChildrenQuery;
 import org.elasticsearch.index.search.child.DeleteByQueryWrappingFilter;
 import org.elasticsearch.index.search.child.ScoreType;
+import org.elasticsearch.index.search.nested.NonNestedDocsFilter;
 import org.elasticsearch.search.internal.SearchContext;
 
 import java.io.IOException;
@@ -133,6 +134,11 @@ public class HasChildQueryParser implements QueryParser {
             throw new QueryParsingException(parseContext.index(), "[has_child]  Type [" + childType + "] points to a non existent parent type [" + parentType + "]");
         }
 
+        Filter nonNestedDocsFilter = null;
+        if (parentDocMapper.hasNestedObjects()) {
+            nonNestedDocsFilter = parseContext.cacheFilter(NonNestedDocsFilter.INSTANCE, null);
+        }
+
         // wrap the query with type query
         innerQuery = new XFilteredQuery(innerQuery, parseContext.cacheFilter(childDocMapper.typeFilter(), null));
 
@@ -140,9 +146,9 @@ public class HasChildQueryParser implements QueryParser {
         Query query;
         Filter parentFilter = parseContext.cacheFilter(parentDocMapper.typeFilter(), null);
         if (!deleteByQuery && scoreType != null) {
-            query = new ChildrenQuery(parentType, childType, parentFilter, innerQuery, scoreType, shortCircuitParentDocSet);
+            query = new ChildrenQuery(parentType, childType, parentFilter, innerQuery, scoreType, shortCircuitParentDocSet, nonNestedDocsFilter);
         } else {
-            query = new ChildrenConstantScoreQuery(innerQuery, parentType, childType, parentFilter, shortCircuitParentDocSet);
+            query = new ChildrenConstantScoreQuery(innerQuery, parentType, childType, parentFilter, shortCircuitParentDocSet, nonNestedDocsFilter);
             if (deleteByQuery) {
                 query = new XConstantScoreQuery(new DeleteByQueryWrappingFilter(query));
             }

+ 4 - 2
src/main/java/org/elasticsearch/index/search/child/ChildrenConstantScoreQuery.java

@@ -53,16 +53,18 @@ public class ChildrenConstantScoreQuery extends Query {
     private final String childType;
     private final Filter parentFilter;
     private final int shortCircuitParentDocSet;
+    private final Filter nonNestedDocsFilter;
 
     private Query rewrittenChildQuery;
     private IndexReader rewriteIndexReader;
 
-    public ChildrenConstantScoreQuery(Query childQuery, String parentType, String childType, Filter parentFilter, int shortCircuitParentDocSet) {
+    public ChildrenConstantScoreQuery(Query childQuery, String parentType, String childType, Filter parentFilter, int shortCircuitParentDocSet, Filter nonNestedDocsFilter) {
         this.parentFilter = parentFilter;
         this.parentType = parentType;
         this.childType = childType;
         this.originalChildQuery = childQuery;
         this.shortCircuitParentDocSet = shortCircuitParentDocSet;
+        this.nonNestedDocsFilter = nonNestedDocsFilter;
     }
 
     @Override
@@ -106,7 +108,7 @@ public class ChildrenConstantScoreQuery extends Query {
             BytesRef id = collectedUids.v().iterator().next().value.toBytesRef();
             shortCircuitFilter = new TermFilter(new Term(UidFieldMapper.NAME, Uid.createUidAsBytes(parentType, id)));
         } else if (remaining <= shortCircuitParentDocSet) {
-            shortCircuitFilter = new ParentIdsFilter(parentType, collectedUids.v().keys, collectedUids.v().allocated);
+            shortCircuitFilter = new ParentIdsFilter(parentType, collectedUids.v().keys, collectedUids.v().allocated, nonNestedDocsFilter);
         }
 
         ParentWeight parentWeight = new ParentWeight(parentFilter, shortCircuitFilter, searchContext, collectedUids);

+ 17 - 4
src/main/java/org/elasticsearch/index/search/child/ChildrenQuery.java

@@ -33,6 +33,7 @@ import org.elasticsearch.ElasticSearchException;
 import org.elasticsearch.common.bytes.HashedBytesArray;
 import org.elasticsearch.common.lease.Releasable;
 import org.elasticsearch.common.lucene.docset.DocIdSets;
+import org.elasticsearch.common.lucene.search.AndFilter;
 import org.elasticsearch.common.lucene.search.ApplyAcceptedDocsFilter;
 import org.elasticsearch.common.lucene.search.Queries;
 import org.elasticsearch.common.recycler.Recycler;
@@ -43,6 +44,8 @@ import org.elasticsearch.index.mapper.internal.UidFieldMapper;
 import org.elasticsearch.search.internal.SearchContext;
 
 import java.io.IOException;
+import java.util.Arrays;
+import java.util.List;
 import java.util.Set;
 
 /**
@@ -62,17 +65,19 @@ public class ChildrenQuery extends Query {
     private final ScoreType scoreType;
     private final Query originalChildQuery;
     private final int shortCircuitParentDocSet;
+    private final Filter nonNestedDocsFilter;
 
     private Query rewrittenChildQuery;
     private IndexReader rewriteIndexReader;
 
-    public ChildrenQuery(String parentType, String childType, Filter parentFilter, Query childQuery, ScoreType scoreType, int shortCircuitParentDocSet) {
+    public ChildrenQuery(String parentType, String childType, Filter parentFilter, Query childQuery, ScoreType scoreType, int shortCircuitParentDocSet, Filter nonNestedDocsFilter) {
         this.parentType = parentType;
         this.childType = childType;
         this.parentFilter = parentFilter;
         this.originalChildQuery = childQuery;
         this.scoreType = scoreType;
         this.shortCircuitParentDocSet = shortCircuitParentDocSet;
+        this.nonNestedDocsFilter = nonNestedDocsFilter;
     }
 
     @Override
@@ -164,12 +169,20 @@ public class ChildrenQuery extends Query {
             return Queries.newMatchNoDocsQuery().createWeight(searcher);
         }
 
-        Filter parentFilter;
+        final Filter parentFilter;
         if (size == 1) {
             BytesRef id = uidToScore.v().keys().iterator().next().value.toBytesRef();
-            parentFilter = new TermFilter(new Term(UidFieldMapper.NAME, Uid.createUidAsBytes(parentType, id)));
+            if (nonNestedDocsFilter != null) {
+                List<Filter> filters = Arrays.asList(
+                        new TermFilter(new Term(UidFieldMapper.NAME, Uid.createUidAsBytes(parentType, id))),
+                        nonNestedDocsFilter
+                );
+                parentFilter = new AndFilter(filters);
+            } else {
+                parentFilter = new TermFilter(new Term(UidFieldMapper.NAME, Uid.createUidAsBytes(parentType, id)));
+            }
         } else if (size <= shortCircuitParentDocSet) {
-            parentFilter = new ParentIdsFilter(parentType, uidToScore.v().keys, uidToScore.v().allocated);
+            parentFilter = new ParentIdsFilter(parentType, uidToScore.v().keys, uidToScore.v().allocated, nonNestedDocsFilter);
         } else {
             parentFilter = new ApplyAcceptedDocsFilter(this.parentFilter);
         }

+ 23 - 5
src/main/java/org/elasticsearch/index/search/child/ParentIdsFilter.java

@@ -48,7 +48,10 @@ final class ParentIdsFilter extends Filter {
     private final Object[] keys;
     private final boolean[] allocated;
 
-    public ParentIdsFilter(String parentType, Object[] keys, boolean[] allocated) {
+    private final Filter nonNestedDocsFilter;
+
+    public ParentIdsFilter(String parentType, Object[] keys, boolean[] allocated, Filter nonNestedDocsFilter) {
+        this.nonNestedDocsFilter = nonNestedDocsFilter;
         this.parentTypeBr = new BytesRef(parentType);
         this.keys = keys;
         this.allocated = allocated;
@@ -65,6 +68,15 @@ final class ParentIdsFilter extends Filter {
         BytesRef uidSpare = new BytesRef();
         BytesRef idSpare = new BytesRef();
 
+        if (acceptDocs == null) {
+            acceptDocs = context.reader().getLiveDocs();
+        }
+
+        FixedBitSet nonNestedDocs = null;
+        if (nonNestedDocsFilter != null) {
+            nonNestedDocs = (FixedBitSet) nonNestedDocsFilter.getDocIdSet(context, acceptDocs);
+        }
+
         DocsEnum docsEnum = null;
         FixedBitSet result = null;
         for (int i = 0; i < allocated.length; i++) {
@@ -82,16 +94,22 @@ final class ParentIdsFilter extends Filter {
                     docId = docsEnum.nextDoc();
                     if (docId != DocIdSetIterator.NO_MORE_DOCS) {
                         result = new FixedBitSet(context.reader().maxDoc());
-                        result.set(docId);
                     } else {
                         continue;
                     }
+                } else {
+                    docId = docsEnum.nextDoc();
+                    if (docId == DocIdSetIterator.NO_MORE_DOCS) {
+                        continue;
+                    }
                 }
-                for (docId = docsEnum.nextDoc(); docId < DocIdSetIterator.NO_MORE_DOCS; docId = docsEnum.nextDoc()) {
-                    result.set(docId);
+                if (nonNestedDocs != null && !nonNestedDocs.get(docId)) {
+                    docId = nonNestedDocs.nextSetBit(docId);
                 }
+                result.set(docId);
+                assert docsEnum.advance(docId + 1) == DocIdSetIterator.NO_MORE_DOCS : "DocId " + docId + " should have been the last one but docId " + docsEnum.docID() + " exists.";
             }
         }
         return result;
     }
-}
+}

+ 5 - 3
src/test/java/org/elasticsearch/index/search/child/ChildrenConstantScoreQueryTests.java

@@ -49,6 +49,7 @@ import org.elasticsearch.index.mapper.Uid;
 import org.elasticsearch.index.mapper.internal.ParentFieldMapper;
 import org.elasticsearch.index.mapper.internal.TypeFieldMapper;
 import org.elasticsearch.index.mapper.internal.UidFieldMapper;
+import org.elasticsearch.index.search.nested.NonNestedDocsFilter;
 import org.elasticsearch.index.service.IndexService;
 import org.elasticsearch.indices.cache.filter.IndicesFilterCache;
 import org.elasticsearch.node.settings.NodeSettingsService;
@@ -108,7 +109,7 @@ public class ChildrenConstantScoreQueryTests extends ElasticsearchLuceneTestCase
         TermQuery childQuery = new TermQuery(new Term("field1", "value" + (1 + random().nextInt(3))));
         TermFilter parentFilter = new TermFilter(new Term(TypeFieldMapper.NAME, "parent"));
         int shortCircuitParentDocSet = random().nextInt(5);
-        ChildrenConstantScoreQuery query = new ChildrenConstantScoreQuery(childQuery, "parent", "child", parentFilter, shortCircuitParentDocSet);
+        ChildrenConstantScoreQuery query = new ChildrenConstantScoreQuery(childQuery, "parent", "child", parentFilter, shortCircuitParentDocSet, null);
 
         BitSetCollector collector = new BitSetCollector(indexReader.maxDoc());
         searcher.search(query, collector);
@@ -248,15 +249,16 @@ public class ChildrenConstantScoreQueryTests extends ElasticsearchLuceneTestCase
             String childValue = childValues[random().nextInt(numUniqueChildValues)];
             TermQuery childQuery = new TermQuery(new Term("field1", childValue));
             int shortCircuitParentDocSet = random().nextInt(numParentDocs);
+            Filter nonNestedDocsFilter = random().nextBoolean() ? NonNestedDocsFilter.INSTANCE : null;
             Query query;
             if (random().nextBoolean()) {
                 // Usage in HasChildQueryParser
-                query = new ChildrenConstantScoreQuery(childQuery, "parent", "child", parentFilter, shortCircuitParentDocSet);
+                query = new ChildrenConstantScoreQuery(childQuery, "parent", "child", parentFilter, shortCircuitParentDocSet, nonNestedDocsFilter);
             } else {
                 // Usage in HasChildFilterParser
                 query = new XConstantScoreQuery(
                         new CustomQueryWrappingFilter(
-                                new ChildrenConstantScoreQuery(childQuery, "parent", "child", parentFilter, shortCircuitParentDocSet)
+                                new ChildrenConstantScoreQuery(childQuery, "parent", "child", parentFilter, shortCircuitParentDocSet, nonNestedDocsFilter)
                         )
                 );
             }

+ 3 - 1
src/test/java/org/elasticsearch/index/search/child/ChildrenQueryTests.java

@@ -36,6 +36,7 @@ import org.elasticsearch.index.mapper.Uid;
 import org.elasticsearch.index.mapper.internal.ParentFieldMapper;
 import org.elasticsearch.index.mapper.internal.TypeFieldMapper;
 import org.elasticsearch.index.mapper.internal.UidFieldMapper;
+import org.elasticsearch.index.search.nested.NonNestedDocsFilter;
 import org.elasticsearch.search.internal.ContextIndexSearcher;
 import org.elasticsearch.search.internal.SearchContext;
 import org.elasticsearch.test.ElasticsearchLuceneTestCase;
@@ -197,7 +198,8 @@ public class ChildrenQueryTests extends ElasticsearchLuceneTestCase {
             Query childQuery = new ConstantScoreQuery(new TermQuery(new Term("field1", childValue)));
             int shortCircuitParentDocSet = random().nextInt(numParentDocs);
             ScoreType scoreType = ScoreType.values()[random().nextInt(ScoreType.values().length)];
-            Query query = new ChildrenQuery("parent", "child", parentFilter, childQuery, scoreType, shortCircuitParentDocSet);
+            Filter nonNestedDocsFilter = random().nextBoolean() ? NonNestedDocsFilter.INSTANCE : null;
+            Query query = new ChildrenQuery("parent", "child", parentFilter, childQuery, scoreType, shortCircuitParentDocSet, nonNestedDocsFilter);
             query = new XFilteredQuery(query, filterMe);
             BitSetCollector collector = new BitSetCollector(indexReader.maxDoc());
             int numHits = 1 + random().nextInt(25);

+ 49 - 0
src/test/java/org/elasticsearch/search/child/SimpleChildQuerySearchTests.java

@@ -35,6 +35,7 @@ import org.elasticsearch.common.lucene.search.function.CombineFunction;
 import org.elasticsearch.common.settings.ImmutableSettings;
 import org.elasticsearch.index.mapper.MergeMappingException;
 import org.elasticsearch.index.query.*;
+import org.elasticsearch.index.search.child.ScoreType;
 import org.elasticsearch.search.facet.terms.TermsFacet;
 import org.elasticsearch.search.sort.SortBuilders;
 import org.elasticsearch.search.sort.SortOrder;
@@ -1955,6 +1956,54 @@ public class SimpleChildQuerySearchTests extends ElasticsearchIntegrationTest {
         }
     }
 
+    @Test
+    public void testHasChildQueryWithNestedInnerObjects() throws Exception {
+        client().admin().indices().prepareCreate("test")
+                .setSettings(
+                        ImmutableSettings.settingsBuilder()
+                                .put("index.number_of_shards", 1)
+                                .put("index.number_of_replicas", 0)
+                )
+                .addMapping("parent", "objects", "type=nested")
+                .addMapping("child", "_parent", "type=parent")
+                .execute().actionGet();
+        client().admin().cluster().prepareHealth().setWaitForEvents(Priority.LANGUID).setWaitForGreenStatus().execute().actionGet();
+
+        client().prepareIndex("test", "parent", "p1")
+                .setSource(jsonBuilder().startObject().field("p_field", "1").startArray("objects")
+                        .startObject().field("i_field", "1").endObject()
+                        .startObject().field("i_field", "2").endObject()
+                        .startObject().field("i_field", "3").endObject()
+                        .startObject().field("i_field", "4").endObject()
+                        .startObject().field("i_field", "5").endObject()
+                        .startObject().field("i_field", "6").endObject()
+                        .endArray().endObject())
+                .execute().actionGet();
+        client().prepareIndex("test", "parent", "p2")
+                .setSource(jsonBuilder().startObject().field("p_field", "2").startArray("objects")
+                        .startObject().field("i_field", "1").endObject()
+                        .startObject().field("i_field", "2").endObject()
+                        .endArray().endObject())
+                .execute().actionGet();
+        client().prepareIndex("test", "child", "c1").setParent("p1").setSource("c_field", "blue").execute().actionGet();
+        client().prepareIndex("test", "child", "c2").setParent("p1").setSource("c_field", "red").execute().actionGet();
+        client().prepareIndex("test", "child", "c3").setParent("p2").setSource("c_field", "red").execute().actionGet();
+        client().admin().indices().prepareRefresh("test").execute().actionGet();
+
+        String scoreMode = ScoreType.values()[randomInt(ScoreType.values().length)].name().toLowerCase(Locale.ROOT);
+        SearchResponse searchResponse = client().prepareSearch("test")
+                .setQuery(filteredQuery(QueryBuilders.hasChildQuery("child", termQuery("c_field", "blue")).scoreType(scoreMode), notFilter(termFilter("p_field", "3"))))
+                .execute().actionGet();
+        assertNoFailures(searchResponse);
+        assertThat(searchResponse.getHits().totalHits(), equalTo(1l));
+
+        searchResponse = client().prepareSearch("test")
+                .setQuery(filteredQuery(QueryBuilders.hasChildQuery("child", termQuery("c_field", "red")).scoreType(scoreMode), notFilter(termFilter("p_field", "3"))))
+                .execute().actionGet();
+        assertNoFailures(searchResponse);
+        assertThat(searchResponse.getHits().totalHits(), equalTo(2l));
+    }
+
     private static HasChildFilterBuilder hasChildFilter(String type, QueryBuilder queryBuilder) {
         HasChildFilterBuilder hasChildFilterBuilder = FilterBuilders.hasChildFilter(type, queryBuilder);
         hasChildFilterBuilder.setShortCircuitCutoff(randomInt(10));