From 82ff1c6802c085201b6a0643a954fd112cd4a88e Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 5 Jun 2013 22:12:26 +0200 Subject: [PATCH] Fixed `has_parent` query and filter returning no results with multi level child docs. --- .../index/query/HasParentFilterParser.java | 15 +++-- .../index/query/HasParentQueryParser.java | 15 +++-- .../child/SimpleChildQuerySearchTests.java | 58 +++++++++++++++++-- 3 files changed, 72 insertions(+), 16 deletions(-) diff --git a/src/main/java/org/elasticsearch/index/query/HasParentFilterParser.java b/src/main/java/org/elasticsearch/index/query/HasParentFilterParser.java index 1f7ef6b61bb..7e669fd5d43 100644 --- a/src/main/java/org/elasticsearch/index/query/HasParentFilterParser.java +++ b/src/main/java/org/elasticsearch/index/query/HasParentFilterParser.java @@ -37,8 +37,8 @@ import org.elasticsearch.index.search.child.HasParentFilter; import org.elasticsearch.search.internal.SearchContext; import java.io.IOException; -import java.util.ArrayList; -import java.util.List; +import java.util.HashSet; +import java.util.Set; /** * @@ -136,17 +136,22 @@ public class HasParentFilterParser implements FilterParser { throw new ElasticSearchIllegalStateException("[has_parent] Can't execute, search context not set"); } - List parentTypes = new ArrayList(2); + Set parentTypes = new HashSet(5); + parentTypes.add(parentType); for (DocumentMapper documentMapper : parseContext.mapperService()) { ParentFieldMapper parentFieldMapper = documentMapper.parentFieldMapper(); if (parentFieldMapper != null) { - parentTypes.add(parentFieldMapper.type()); + DocumentMapper parentTypeDocumentMapper = searchContext.mapperService().documentMapper(parentFieldMapper.type()); + if (parentTypeDocumentMapper == null) { + // Only add this, if this parentFieldMapper (also a parent) isn't a child of another parent. + parentTypes.add(parentFieldMapper.type()); + } } } Filter parentFilter; if (parentTypes.size() == 1) { - DocumentMapper documentMapper = parseContext.mapperService().documentMapper(parentTypes.get(0)); + DocumentMapper documentMapper = parseContext.mapperService().documentMapper(parentTypes.iterator().next()); parentFilter = parseContext.cacheFilter(documentMapper.typeFilter(), null); } else { XBooleanFilter parentsFilter = new XBooleanFilter(); diff --git a/src/main/java/org/elasticsearch/index/query/HasParentQueryParser.java b/src/main/java/org/elasticsearch/index/query/HasParentQueryParser.java index 7067568ad2a..798dd89002c 100644 --- a/src/main/java/org/elasticsearch/index/query/HasParentQueryParser.java +++ b/src/main/java/org/elasticsearch/index/query/HasParentQueryParser.java @@ -37,8 +37,8 @@ import org.elasticsearch.index.search.child.ParentQuery; import org.elasticsearch.search.internal.SearchContext; import java.io.IOException; -import java.util.ArrayList; -import java.util.List; +import java.util.HashSet; +import java.util.Set; public class HasParentQueryParser implements QueryParser { @@ -131,17 +131,22 @@ public class HasParentQueryParser implements QueryParser { throw new ElasticSearchIllegalStateException("[has_parent] Can't execute, search context not set."); } - List parentTypes = new ArrayList(2); + Set parentTypes = new HashSet(5); + parentTypes.add(parentType); for (DocumentMapper documentMapper : parseContext.mapperService()) { ParentFieldMapper parentFieldMapper = documentMapper.parentFieldMapper(); if (parentFieldMapper != null) { - parentTypes.add(parentFieldMapper.type()); + DocumentMapper parentTypeDocumentMapper = searchContext.mapperService().documentMapper(parentFieldMapper.type()); + if (parentTypeDocumentMapper == null) { + // Only add this, if this parentFieldMapper (also a parent) isn't a child of another parent. + parentTypes.add(parentFieldMapper.type()); + } } } Filter parentFilter; if (parentTypes.size() == 1) { - DocumentMapper documentMapper = parseContext.mapperService().documentMapper(parentTypes.get(0)); + DocumentMapper documentMapper = parseContext.mapperService().documentMapper(parentTypes.iterator().next()); parentFilter = parseContext.cacheFilter(documentMapper.typeFilter(), null); } else { XBooleanFilter parentsFilter = new XBooleanFilter(); diff --git a/src/test/java/org/elasticsearch/test/integration/search/child/SimpleChildQuerySearchTests.java b/src/test/java/org/elasticsearch/test/integration/search/child/SimpleChildQuerySearchTests.java index 791f51fa918..f31e5b07099 100644 --- a/src/test/java/org/elasticsearch/test/integration/search/child/SimpleChildQuerySearchTests.java +++ b/src/test/java/org/elasticsearch/test/integration/search/child/SimpleChildQuerySearchTests.java @@ -31,15 +31,10 @@ import org.elasticsearch.search.facet.terms.TermsFacet; import org.elasticsearch.search.sort.SortBuilders; import org.elasticsearch.search.sort.SortOrder; import org.elasticsearch.test.integration.AbstractSharedClusterTest; -import org.testng.annotations.BeforeTest; import org.testng.annotations.Test; import java.io.IOException; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import java.util.Locale; -import java.util.Map; +import java.util.*; import static com.google.common.collect.Maps.newHashMap; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; @@ -91,6 +86,57 @@ public class SimpleChildQuerySearchTests extends AbstractSharedClusterTest { .execute().actionGet(); assertThat("Failures " + Arrays.toString(searchResponse.getShardFailures()), searchResponse.getShardFailures().length, equalTo(0)); assertThat(searchResponse.getHits().totalHits(), equalTo(1l)); + assertThat(searchResponse.getHits().getAt(0).id(), equalTo("p1")); + + searchResponse = client().prepareSearch("test") + .setQuery( + filteredQuery( + matchAllQuery(), + hasParentFilter( + "parent", + termFilter("p_field", "p_value1") + ) + ) + ).execute().actionGet(); + assertThat("Failures " + Arrays.toString(searchResponse.getShardFailures()), searchResponse.getShardFailures().length, equalTo(0)); + assertThat(searchResponse.getHits().totalHits(), equalTo(1l)); + assertThat(searchResponse.getHits().getAt(0).id(), equalTo("c1")); + + searchResponse = client().prepareSearch("test") + .setQuery( + filteredQuery( + matchAllQuery(), + hasParentFilter( + "child", + termFilter("c_field", "c_value1") + ) + ) + ).execute().actionGet(); + assertThat("Failures " + Arrays.toString(searchResponse.getShardFailures()), searchResponse.getShardFailures().length, equalTo(0)); + assertThat(searchResponse.getHits().totalHits(), equalTo(1l)); + assertThat(searchResponse.getHits().getAt(0).id(), equalTo("gc1")); + + searchResponse = client().prepareSearch("test") + .setQuery( + hasParentQuery( + "parent", + termQuery("p_field", "p_value1") + ) + ).execute().actionGet(); + assertThat("Failures " + Arrays.toString(searchResponse.getShardFailures()), searchResponse.getShardFailures().length, equalTo(0)); + assertThat(searchResponse.getHits().totalHits(), equalTo(1l)); + assertThat(searchResponse.getHits().getAt(0).id(), equalTo("c1")); + + searchResponse = client().prepareSearch("test") + .setQuery( + hasParentQuery( + "child", + termQuery("c_field", "c_value1") + ) + ).execute().actionGet(); + assertThat("Failures " + Arrays.toString(searchResponse.getShardFailures()), searchResponse.getShardFailures().length, equalTo(0)); + assertThat(searchResponse.getHits().totalHits(), equalTo(1l)); + assertThat(searchResponse.getHits().getAt(0).id(), equalTo("gc1")); }