From a4e08acdd1dae9b9172ba3b83addec6bb6fabfa9 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Thu, 2 Jul 2020 21:51:28 +0200 Subject: [PATCH] Fix exists query on unmapped field in query_string (#58804) Since #55785, exists queries rewrite to MatchNoneQueryBuilder when the field is unmapped. This change also introduced a bug in the `query_string` query, using an unmapped field like `_exists_:foo` throws an exception if the field is unmapped. This commit avoids the exception if the query is built outside of an `ExistsQueryBuilder`. Closes #58737 --- .../elasticsearch/index/query/ExistsQueryBuilder.java | 11 ++++++++--- .../index/query/LegacyGeoShapeQueryProcessor.java | 2 +- .../elasticsearch/index/query/RangeQueryBuilder.java | 2 +- .../index/search/QueryStringQueryParser.java | 2 +- .../index/query/ExistsQueryBuilderTests.java | 2 ++ 5 files changed, 13 insertions(+), 6 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 926a3fa56ec..8f77221fd98 100644 --- a/server/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/ExistsQueryBuilder.java @@ -24,6 +24,7 @@ import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanClause.Occur; import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.ConstantScoreQuery; +import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.TermQuery; import org.elasticsearch.Version; @@ -139,15 +140,19 @@ public class ExistsQueryBuilder extends AbstractQueryBuilder @Override protected Query doToQuery(QueryShardContext context) throws IOException { - return newFilter(context, fieldName); + return newFilter(context, fieldName, true); } - public static Query newFilter(QueryShardContext context, String fieldPattern) { + public static Query newFilter(QueryShardContext context, String fieldPattern, boolean checkRewrite) { Collection fields = getMappedField(context, fieldPattern); if (fields.isEmpty()) { - throw new IllegalStateException("Rewrite first"); + if (checkRewrite) { + throw new IllegalStateException("Rewrite first"); + } else { + return new MatchNoDocsQuery("unmapped field:" + fieldPattern); + } } if (context.indexVersionCreated().before(Version.V_6_1_0)) { diff --git a/server/src/main/java/org/elasticsearch/index/query/LegacyGeoShapeQueryProcessor.java b/server/src/main/java/org/elasticsearch/index/query/LegacyGeoShapeQueryProcessor.java index 0778e912513..a8c5c2e70b0 100644 --- a/server/src/main/java/org/elasticsearch/index/query/LegacyGeoShapeQueryProcessor.java +++ b/server/src/main/java/org/elasticsearch/index/query/LegacyGeoShapeQueryProcessor.java @@ -94,7 +94,7 @@ public class LegacyGeoShapeQueryProcessor implements QueryProcessor { // before, including creating lucene fieldcache (!) // in this case, execute disjoint as exists && !intersects BooleanQuery.Builder bool = new BooleanQuery.Builder(); - Query exists = ExistsQueryBuilder.newFilter(context, fieldName); + Query exists = ExistsQueryBuilder.newFilter(context, fieldName,false); Query intersects = prefixTreeStrategy.makeQuery(getArgs(shape, ShapeRelation.INTERSECTS)); bool.add(exists, BooleanClause.Occur.MUST); bool.add(intersects, BooleanClause.Occur.MUST_NOT); diff --git a/server/src/main/java/org/elasticsearch/index/query/RangeQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/RangeQueryBuilder.java index 6cb1704611b..e8482866e7d 100644 --- a/server/src/main/java/org/elasticsearch/index/query/RangeQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/RangeQueryBuilder.java @@ -486,7 +486,7 @@ public class RangeQueryBuilder extends AbstractQueryBuilder i } // Exists query would fail if the fieldNames field is disabled. if (fieldNamesFieldType.isEnabled()) { - return ExistsQueryBuilder.newFilter(context, fieldName); + return ExistsQueryBuilder.newFilter(context, fieldName, false); } } MappedFieldType mapper = context.fieldMapper(this.fieldName); diff --git a/server/src/main/java/org/elasticsearch/index/search/QueryStringQueryParser.java b/server/src/main/java/org/elasticsearch/index/search/QueryStringQueryParser.java index 35133f31a19..4260df0a6e6 100644 --- a/server/src/main/java/org/elasticsearch/index/search/QueryStringQueryParser.java +++ b/server/src/main/java/org/elasticsearch/index/search/QueryStringQueryParser.java @@ -632,7 +632,7 @@ public class QueryStringQueryParser extends XQueryParser { return new WildcardQuery(new Term(fieldName, "*")); } - return ExistsQueryBuilder.newFilter(context, fieldName); + return ExistsQueryBuilder.newFilter(context, fieldName, false); } @Override 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 d4402843b7d..db0ba65c653 100644 --- a/server/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/ExistsQueryBuilderTests.java @@ -134,6 +134,8 @@ public class ExistsQueryBuilderTests extends AbstractQueryTestCase queryBuilder.toQuery(context)); assertEquals("Rewrite first", e.getMessage()); + Query ret = ExistsQueryBuilder.newFilter(context, "foo", false); + assertThat(ret, instanceOf(MatchNoDocsQuery.class)); } public void testIllegalArguments() {