From 98c164c3f00fb3bf08cc7063184a6151a95ddbcf Mon Sep 17 00:00:00 2001 From: Alexander Kazakov Date: Thu, 3 Mar 2016 13:32:22 +0300 Subject: [PATCH] Check that parent_type in HasParent query has child types #16692 --- .../index/query/HasParentQueryBuilder.java | 49 +++++++------------ .../search/child/ChildQuerySearchIT.java | 22 ++++++--- 2 files changed, 33 insertions(+), 38 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/HasParentQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/HasParentQueryBuilder.java index 281bc061170..da26fa90b35 100644 --- a/core/src/main/java/org/elasticsearch/index/query/HasParentQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/HasParentQueryBuilder.java @@ -132,7 +132,7 @@ public class HasParentQueryBuilder extends AbstractQueryBuilder parentTypes = new HashSet<>(5); - parentTypes.add(parentDocMapper.type()); + Set childTypes = new HashSet<>(); ParentChildIndexFieldData parentChildIndexFieldData = null; for (DocumentMapper documentMapper : context.getMapperService().docMappers(false)) { ParentFieldMapper parentFieldMapper = documentMapper.parentFieldMapper(); - if (parentFieldMapper.active()) { - DocumentMapper parentTypeDocumentMapper = context.getMapperService().documentMapper(parentFieldMapper.type()); + if (parentFieldMapper.active() && type.equals(parentFieldMapper.type())) { + childTypes.add(documentMapper.type()); parentChildIndexFieldData = context.getForField(parentFieldMapper.fieldType()); - if (parentTypeDocumentMapper == null) { - // Only add this, if this parentFieldMapper (also a parent) isn't a child of another parent. - parentTypes.add(parentFieldMapper.type()); - } } } - if (parentChildIndexFieldData == null) { - throw new QueryShardException(context, "[has_parent] no _parent field configured"); - } - Query parentTypeQuery = null; - if (parentTypes.size() == 1) { - DocumentMapper documentMapper = context.getMapperService().documentMapper(parentTypes.iterator().next()); - if (documentMapper != null) { - parentTypeQuery = documentMapper.typeFilter(); - } + if (childTypes.isEmpty()) { + throw new QueryShardException(context, "[" + NAME + "] no child types found for type [" + type + "]"); + } + + Query childrenQuery; + if (childTypes.size() == 1) { + DocumentMapper documentMapper = context.getMapperService().documentMapper(childTypes.iterator().next()); + childrenQuery = documentMapper.typeFilter(); } else { - BooleanQuery.Builder parentsFilter = new BooleanQuery.Builder(); - for (String parentTypeStr : parentTypes) { - DocumentMapper documentMapper = context.getMapperService().documentMapper(parentTypeStr); - if (documentMapper != null) { - parentsFilter.add(documentMapper.typeFilter(), BooleanClause.Occur.SHOULD); - } + BooleanQuery.Builder childrenFilter = new BooleanQuery.Builder(); + for (String childrenTypeStr : childTypes) { + DocumentMapper documentMapper = context.getMapperService().documentMapper(childrenTypeStr); + childrenFilter.add(documentMapper.typeFilter(), BooleanClause.Occur.SHOULD); } - parentTypeQuery = parentsFilter.build(); - } - - if (parentTypeQuery == null) { - return null; + childrenQuery = childrenFilter.build(); } // wrap the query with type query innerQuery = Queries.filtered(innerQuery, parentDocMapper.typeFilter()); - Query childrenFilter = Queries.not(parentTypeQuery); - return new HasChildQueryBuilder.LateParsingQuery(childrenFilter, + return new HasChildQueryBuilder.LateParsingQuery(childrenQuery, innerQuery, HasChildQueryBuilder.DEFAULT_MIN_CHILDREN, HasChildQueryBuilder.DEFAULT_MAX_CHILDREN, diff --git a/core/src/test/java/org/elasticsearch/search/child/ChildQuerySearchIT.java b/core/src/test/java/org/elasticsearch/search/child/ChildQuerySearchIT.java index d74a148a2f9..6300d5d355e 100644 --- a/core/src/test/java/org/elasticsearch/search/child/ChildQuerySearchIT.java +++ b/core/src/test/java/org/elasticsearch/search/child/ChildQuerySearchIT.java @@ -758,11 +758,11 @@ public class ChildQuerySearchIT extends ESIntegTestCase { assertNoFailures(response); assertThat(response.getHits().totalHits(), equalTo(0L)); - response = client().prepareSearch("test").setQuery(QueryBuilders.hasParentQuery("child", matchQuery("text", "value"))).get(); + response = client().prepareSearch("test").setQuery(QueryBuilders.hasParentQuery("parent", matchQuery("text", "value"))).get(); assertNoFailures(response); assertThat(response.getHits().totalHits(), equalTo(0L)); - response = client().prepareSearch("test").setQuery(QueryBuilders.hasParentQuery("child", matchQuery("text", "value")).score(true)) + response = client().prepareSearch("test").setQuery(QueryBuilders.hasParentQuery("parent", matchQuery("text", "value")).score(true)) .get(); assertNoFailures(response); assertThat(response.getHits().totalHits(), equalTo(0L)); @@ -1894,11 +1894,6 @@ public class ChildQuerySearchIT extends ESIntegTestCase { fail(); } catch (SearchPhaseExecutionException e) { } - - SearchResponse response = client().prepareSearch("test") - .setQuery(QueryBuilders.hasParentQuery("parent", matchAllQuery())) - .get(); - assertHitCount(response, 0); } static HasChildQueryBuilder hasChildQuery(String type, QueryBuilder queryBuilder) { @@ -1927,4 +1922,17 @@ public class ChildQuerySearchIT extends ESIntegTestCase { QueryBuilders.hasChildQuery("child-type", new IdsQueryBuilder().addIds("child-id"))).get(); assertSearchHits(searchResponse, "parent-id"); } + + public void testParentWithoutChildTypes() { + assertAcked(prepareCreate("test").addMapping("parent").addMapping("child", "_parent", "type=parent")); + ensureGreen(); + + try { + client().prepareSearch("test").setQuery(hasParentQuery("child", matchAllQuery())).get(); + fail(); + } catch (SearchPhaseExecutionException e) { + assertThat(e.status(), equalTo(RestStatus.BAD_REQUEST)); + assertThat(e.toString(), containsString("[has_parent] no child types found for type [child]")); + } + } }