From 0753d9a35c21a84671ed0b892ba9f5d5eac8b0b0 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Mon, 27 Apr 2020 11:06:50 +0200 Subject: [PATCH] Exists queries to MatchNoneQueryBuilder when the field is unmapped (#55785) Co-authored-by: Sivagurunathan Velayutham Closes #54062 --- .../index/query/ExistsQueryBuilder.java | 67 +++++++++++++++---- .../index/query/ExistsQueryBuilderTests.java | 15 +++-- 2 files changed, 64 insertions(+), 18 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java index 3b44ead98de..926a3fa56ec 100644 --- a/server/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java @@ -80,6 +80,18 @@ public class ExistsQueryBuilder extends AbstractQueryBuilder return this.fieldName; } + @Override + protected QueryBuilder doRewrite(QueryRewriteContext queryShardContext) throws IOException { + QueryShardContext context = queryShardContext.convertToShardContext(); + if (context != null) { + Collection fields = getMappedField(context, fieldName); + if (fields.isEmpty()) { + return new MatchNoneQueryBuilder(); + } + } + return super.doRewrite(queryShardContext); + } + @Override protected void doXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(NAME); @@ -132,20 +144,10 @@ public class ExistsQueryBuilder extends AbstractQueryBuilder public static Query newFilter(QueryShardContext context, String fieldPattern) { - final FieldNamesFieldMapper.FieldNamesFieldType fieldNamesFieldType = (FieldNamesFieldMapper.FieldNamesFieldType) context - .getMapperService().fieldType(FieldNamesFieldMapper.NAME); - if (fieldNamesFieldType == null) { - // can only happen when no types exist, so no docs exist either - return Queries.newMatchNoDocsQuery("Missing types in \"" + NAME + "\" query."); - } + Collection fields = getMappedField(context, fieldPattern); - final Collection fields; - if (context.getObjectMapper(fieldPattern) != null) { - // the _field_names field also indexes objects, so we don't have to - // do any more work to support exists queries on whole objects - fields = Collections.singleton(fieldPattern); - } else { - fields = context.simpleMatchToIndexNames(fieldPattern); + if (fields.isEmpty()) { + throw new IllegalStateException("Rewrite first"); } if (context.indexVersionCreated().before(Version.V_6_1_0)) { @@ -194,7 +196,7 @@ public class ExistsQueryBuilder extends AbstractQueryBuilder if (context.getObjectMapper(field) != null) { return newObjectFieldExistsQuery(context, field); } - return Queries.newMatchNoDocsQuery("No field \"" + field + "\" exists in mappings."); + return Queries.newMatchNoDocsQuery("User requested \"match_none\" query."); } Query filter = fieldType.existsQuery(context); return new ConstantScoreQuery(filter); @@ -210,6 +212,43 @@ public class ExistsQueryBuilder extends AbstractQueryBuilder return new ConstantScoreQuery(booleanQuery.build()); } + /** + * Helper method to get field mapped to this fieldPattern + * @return return collection of fields if exists else return empty. + */ + private static Collection getMappedField(QueryShardContext context, String fieldPattern) { + final FieldNamesFieldMapper.FieldNamesFieldType fieldNamesFieldType = (FieldNamesFieldMapper.FieldNamesFieldType) context + .getMapperService().fieldType(FieldNamesFieldMapper.NAME); + + if (fieldNamesFieldType == null) { + // can only happen when no types exist, so no docs exist either + return Collections.emptySet(); + } + + final Collection fields; + if (context.getObjectMapper(fieldPattern) != null) { + // the _field_names field also indexes objects, so we don't have to + // do any more work to support exists queries on whole objects + fields = Collections.singleton(fieldPattern); + } else { + fields = context.simpleMatchToIndexNames(fieldPattern); + } + + if (fields.size() == 1) { + String field = fields.iterator().next(); + MappedFieldType fieldType = context.getMapperService().fieldType(field); + if (fieldType == null) { + // The field does not exist as a leaf but could be an object so + // check for an object mapper + if (context.getObjectMapper(field) == null) { + return Collections.emptySet(); + } + } + } + + return fields; + } + @Override protected int doHashCode() { return Objects.hash(fieldName); diff --git a/server/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTests.java index 62281bd1ce4..206681266e7 100644 --- a/server/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTests.java @@ -36,7 +36,6 @@ import java.util.Collection; import java.util.List; import java.util.stream.Collectors; -import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.instanceOf; @@ -83,11 +82,9 @@ public class ExistsQueryBuilderTests extends AbstractQueryTestCase queryBuilder.toQuery(context)); + assertEquals("Rewrite first", e.getMessage()); + } + public void testIllegalArguments() { expectThrows(IllegalArgumentException.class, () -> new ExistsQueryBuilder((String) null)); expectThrows(IllegalArgumentException.class, () -> new ExistsQueryBuilder(""));