From c01eecc8e0afb5c27e787c7045d6d3148ca19fa3 Mon Sep 17 00:00:00 2001 From: Alex Ksikes Date: Thu, 2 Jul 2015 23:14:26 -0500 Subject: [PATCH] Refactoring of MissingQuery Relates to #10217 Closes #12030 This PR is against the query-refactoring branch. --- .../classic/MissingFieldQueryExtension.java | 3 +- .../index/query/MissingQueryBuilder.java | 192 ++++++++++++++++-- .../index/query/MissingQueryParser.java | 122 +---------- .../index/query/MissingQueryBuilderTest.java | 184 +++++++++++++++++ 4 files changed, 372 insertions(+), 129 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/index/query/MissingQueryBuilderTest.java diff --git a/core/src/main/java/org/apache/lucene/queryparser/classic/MissingFieldQueryExtension.java b/core/src/main/java/org/apache/lucene/queryparser/classic/MissingFieldQueryExtension.java index ead4e34e68860..8ec3bfbe75e58 100644 --- a/core/src/main/java/org/apache/lucene/queryparser/classic/MissingFieldQueryExtension.java +++ b/core/src/main/java/org/apache/lucene/queryparser/classic/MissingFieldQueryExtension.java @@ -21,6 +21,7 @@ import org.apache.lucene.search.ConstantScoreQuery; import org.apache.lucene.search.Query; +import org.elasticsearch.index.query.MissingQueryBuilder; import org.elasticsearch.index.query.MissingQueryParser; import org.elasticsearch.index.query.QueryParseContext; @@ -33,7 +34,7 @@ public class MissingFieldQueryExtension implements FieldQueryExtension { @Override public Query query(QueryParseContext parseContext, String queryText) { - Query query = MissingQueryParser.newFilter(parseContext, queryText, MissingQueryParser.DEFAULT_EXISTENCE_VALUE, MissingQueryParser.DEFAULT_NULL_VALUE); + Query query = MissingQueryBuilder.newFilter(parseContext, queryText, MissingQueryBuilder.DEFAULT_EXISTENCE_VALUE, MissingQueryBuilder.DEFAULT_NULL_VALUE); if (query != null) { return new ConstantScoreQuery(query); } diff --git a/core/src/main/java/org/elasticsearch/index/query/MissingQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/MissingQueryBuilder.java index 1d8cb60c47fa9..61330eb017c1a 100644 --- a/core/src/main/java/org/elasticsearch/index/query/MissingQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/MissingQueryBuilder.java @@ -19,29 +19,47 @@ package org.elasticsearch.index.query; +import org.apache.lucene.search.*; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.index.mapper.MappedFieldType; +import org.elasticsearch.index.mapper.internal.FieldNamesFieldMapper; +import org.elasticsearch.index.mapper.object.ObjectMapper; import java.io.IOException; +import java.util.Collection; +import java.util.Objects; /** - * Constructs a filter that only match on documents that the field has a value in them. + * Constructs a filter that have only null values or no value in the original field. */ public class MissingQueryBuilder extends AbstractQueryBuilder { public static final String NAME = "missing"; - private String name; + public static final boolean DEFAULT_NULL_VALUE = false; - private Boolean nullValue; + public static final boolean DEFAULT_EXISTENCE_VALUE = true; - private Boolean existence; + private final String fieldPattern; + + private boolean nullValue = DEFAULT_NULL_VALUE; + + private boolean existence = DEFAULT_EXISTENCE_VALUE; static final MissingQueryBuilder PROTOTYPE = new MissingQueryBuilder(null); - public MissingQueryBuilder(String name) { - this.name = name; + public MissingQueryBuilder(String fieldPattern) { + this.fieldPattern = fieldPattern; } - + + public String fieldPattern() { + return this.fieldPattern; + } + /** * Should the missing filter automatically include fields with null value configured in the * mappings. Defaults to false. @@ -52,7 +70,15 @@ public MissingQueryBuilder nullValue(boolean nullValue) { } /** - * Should the missing filter include documents where the field doesn't exists in the docs. + * Returns true if the missing filter will include documents where the field contains a null value, otherwise + * these documents will not be included. + */ + public boolean nullValue() { + return this.nullValue; + } + + /** + * Should the missing filter include documents where the field doesn't exist in the docs. * Defaults to true. */ public MissingQueryBuilder existence(boolean existence) { @@ -60,16 +86,20 @@ public MissingQueryBuilder existence(boolean existence) { return this; } + /** + * Returns true if the missing filter will include documents where the field has no values, otherwise + * these documents will not be included. + */ + public boolean existence() { + return this.existence; + } + @Override protected void doXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(NAME); - builder.field("field", name); - if (nullValue != null) { - builder.field("null_value", nullValue); - } - if (existence != null) { - builder.field("existence", existence); - } + builder.field("field", fieldPattern); + builder.field("null_value", nullValue); + builder.field("existence", existence); printBoostAndQueryName(builder); builder.endObject(); } @@ -78,4 +108,136 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep public String getName() { return NAME; } + + @Override + protected Query doToQuery(QueryParseContext parseContext) throws IOException { + return newFilter(parseContext, fieldPattern, existence, nullValue); + } + + public static Query newFilter(QueryParseContext parseContext, String fieldPattern, boolean existence, boolean nullValue) { + if (!existence && !nullValue) { + throw new QueryParsingException(parseContext, "missing must have either existence, or null_value, or both set to true"); + } + + final FieldNamesFieldMapper.FieldNamesFieldType fieldNamesFieldType = (FieldNamesFieldMapper.FieldNamesFieldType) parseContext.mapperService().fullName(FieldNamesFieldMapper.NAME); + if (fieldNamesFieldType == null) { + // can only happen when no types exist, so no docs exist either + return Queries.newMatchNoDocsQuery(); + } + + ObjectMapper objectMapper = parseContext.getObjectMapper(fieldPattern); + if (objectMapper != null) { + // automatic make the object mapper pattern + fieldPattern = fieldPattern + ".*"; + } + + Collection fields = parseContext.simpleMatchToIndexNames(fieldPattern); + if (fields.isEmpty()) { + if (existence) { + // if we ask for existence of fields, and we found none, then we should match on all + return Queries.newMatchAllQuery(); + } + return null; + } + + Query existenceFilter = null; + Query nullFilter = null; + + if (existence) { + BooleanQuery boolFilter = new BooleanQuery(); + for (String field : fields) { + MappedFieldType fieldType = parseContext.fieldMapper(field); + Query filter = null; + if (fieldNamesFieldType.isEnabled()) { + final String f; + if (fieldType != null) { + f = fieldType.names().indexName(); + } else { + f = field; + } + filter = fieldNamesFieldType.termQuery(f, parseContext); + } + // if _field_names are not indexed, we need to go the slow way + if (filter == null && fieldType != null) { + filter = fieldType.rangeQuery(null, null, true, true, parseContext); + } + if (filter == null) { + filter = new TermRangeQuery(field, null, null, true, true); + } + boolFilter.add(filter, BooleanClause.Occur.SHOULD); + } + + existenceFilter = boolFilter; + existenceFilter = Queries.not(existenceFilter);; + } + + if (nullValue) { + for (String field : fields) { + MappedFieldType fieldType = parseContext.fieldMapper(field); + if (fieldType != null) { + nullFilter = fieldType.nullValueQuery(); + } + } + } + + Query filter; + if (nullFilter != null) { + if (existenceFilter != null) { + BooleanQuery combined = new BooleanQuery(); + combined.add(existenceFilter, BooleanClause.Occur.SHOULD); + combined.add(nullFilter, BooleanClause.Occur.SHOULD); + // cache the not filter as well, so it will be faster + filter = combined; + } else { + filter = nullFilter; + } + } else { + filter = existenceFilter; + } + + if (filter == null) { + return null; + } + + return new ConstantScoreQuery(filter); + } + + @Override + public QueryValidationException validate() { + QueryValidationException validationException = null; + if (Strings.isEmpty(this.fieldPattern)) { + validationException = addValidationError("missing must be provided with a [field]", validationException); + } + if (!existence && !nullValue) { + validationException = addValidationError("missing must have either existence, or null_value, or both set to true", validationException); + } + return validationException; + } + + @Override + protected MissingQueryBuilder doReadFrom(StreamInput in) throws IOException { + MissingQueryBuilder missingQueryBuilder = new MissingQueryBuilder(in.readString()); + missingQueryBuilder.nullValue = in.readBoolean(); + missingQueryBuilder.existence = in.readBoolean(); + return missingQueryBuilder; + } + + @Override + protected void doWriteTo(StreamOutput out) throws IOException { + out.writeString(fieldPattern); + out.writeBoolean(nullValue); + out.writeBoolean(existence); + } + + @Override + protected int doHashCode() { + return Objects.hash(fieldPattern, nullValue, existence); + } + + @Override + protected boolean doEquals(MissingQueryBuilder other) { + return Objects.equals(fieldPattern, other.fieldPattern) && + Objects.equals(nullValue, other.nullValue) && + Objects.equals(existence, other.existence); + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/MissingQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/MissingQueryParser.java index 13cbcbefb7c05..bf00360241ac3 100644 --- a/core/src/main/java/org/elasticsearch/index/query/MissingQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/MissingQueryParser.java @@ -19,28 +19,15 @@ package org.elasticsearch.index.query; -import org.apache.lucene.search.BooleanClause; -import org.apache.lucene.search.BooleanQuery; -import org.apache.lucene.search.ConstantScoreQuery; -import org.apache.lucene.search.Query; -import org.apache.lucene.search.TermRangeQuery; import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.index.mapper.MappedFieldType; -import org.elasticsearch.index.mapper.internal.FieldNamesFieldMapper; -import org.elasticsearch.index.mapper.object.ObjectMapper; import java.io.IOException; -import java.util.Collection; /** * */ -public class MissingQueryParser extends BaseQueryParserTemp { - - public static final boolean DEFAULT_NULL_VALUE = false; - public static final boolean DEFAULT_EXISTENCE_VALUE = true; +public class MissingQueryParser extends BaseQueryParser { @Inject public MissingQueryParser() { @@ -52,14 +39,14 @@ public String[] names() { } @Override - public Query parse(QueryParseContext parseContext) throws IOException, QueryParsingException { + public QueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException { XContentParser parser = parseContext.parser(); String fieldPattern = null; String queryName = null; float boost = AbstractQueryBuilder.DEFAULT_BOOST; - boolean nullValue = DEFAULT_NULL_VALUE; - boolean existence = DEFAULT_EXISTENCE_VALUE; + boolean nullValue = MissingQueryBuilder.DEFAULT_NULL_VALUE; + boolean existence = MissingQueryBuilder.DEFAULT_EXISTENCE_VALUE; XContentParser.Token token; String currentFieldName = null; @@ -86,102 +73,11 @@ public Query parse(QueryParseContext parseContext) throws IOException, QueryPars if (fieldPattern == null) { throw new QueryParsingException(parseContext, "missing must be provided with a [field]"); } - Query query = newFilter(parseContext, fieldPattern, existence, nullValue); - if (queryName != null) { - parseContext.addNamedQuery(queryName, query); - } - if (query != null) { - query.setBoost(boost); - } - return query; - } - - public static Query newFilter(QueryParseContext parseContext, String fieldPattern, boolean existence, boolean nullValue) { - if (!existence && !nullValue) { - throw new QueryParsingException(parseContext, "missing must have either existence, or null_value, or both set to true"); - } - - final FieldNamesFieldMapper.FieldNamesFieldType fieldNamesFieldType = (FieldNamesFieldMapper.FieldNamesFieldType)parseContext.mapperService().fullName(FieldNamesFieldMapper.NAME); - if (fieldNamesFieldType == null) { - // can only happen when no types exist, so no docs exist either - return Queries.newMatchNoDocsQuery(); - } - - ObjectMapper objectMapper = parseContext.getObjectMapper(fieldPattern); - if (objectMapper != null) { - // automatic make the object mapper pattern - fieldPattern = fieldPattern + ".*"; - } - - Collection fields = parseContext.simpleMatchToIndexNames(fieldPattern); - if (fields.isEmpty()) { - if (existence) { - // if we ask for existence of fields, and we found none, then we should match on all - return Queries.newMatchAllQuery(); - } - return null; - } - - Query existenceFilter = null; - Query nullFilter = null; - - if (existence) { - BooleanQuery boolFilter = new BooleanQuery(); - for (String field : fields) { - MappedFieldType fieldType = parseContext.fieldMapper(field); - Query filter = null; - if (fieldNamesFieldType.isEnabled()) { - final String f; - if (fieldType != null) { - f = fieldType.names().indexName(); - } else { - f = field; - } - filter = fieldNamesFieldType.termQuery(f, parseContext); - } - // if _field_names are not indexed, we need to go the slow way - if (filter == null && fieldType != null) { - filter = fieldType.rangeQuery(null, null, true, true, parseContext); - } - if (filter == null) { - filter = new TermRangeQuery(field, null, null, true, true); - } - boolFilter.add(filter, BooleanClause.Occur.SHOULD); - } - - existenceFilter = boolFilter; - existenceFilter = Queries.not(existenceFilter);; - } - - if (nullValue) { - for (String field : fields) { - MappedFieldType fieldType = parseContext.fieldMapper(field); - if (fieldType != null) { - nullFilter = fieldType.nullValueQuery(); - } - } - } - - Query filter; - if (nullFilter != null) { - if (existenceFilter != null) { - BooleanQuery combined = new BooleanQuery(); - combined.add(existenceFilter, BooleanClause.Occur.SHOULD); - combined.add(nullFilter, BooleanClause.Occur.SHOULD); - // cache the not filter as well, so it will be faster - filter = combined; - } else { - filter = nullFilter; - } - } else { - filter = existenceFilter; - } - - if (filter == null) { - return null; - } - - return new ConstantScoreQuery(filter); + return new MissingQueryBuilder(fieldPattern) + .nullValue(nullValue) + .existence(existence) + .boost(boost) + .queryName(queryName); } @Override diff --git a/core/src/test/java/org/elasticsearch/index/query/MissingQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/MissingQueryBuilderTest.java new file mode 100644 index 0000000000000..7155024f4e0ce --- /dev/null +++ b/core/src/test/java/org/elasticsearch/index/query/MissingQueryBuilderTest.java @@ -0,0 +1,184 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.query; + +import org.apache.lucene.search.*; +import org.elasticsearch.common.lucene.search.Queries; +import org.elasticsearch.index.mapper.MappedFieldType; +import org.elasticsearch.index.mapper.internal.FieldNamesFieldMapper; +import org.elasticsearch.index.mapper.object.ObjectMapper; +import org.junit.Test; + +import java.io.IOException; +import java.util.Collection; + +import static org.hamcrest.Matchers.is; + +public class MissingQueryBuilderTest extends BaseQueryTestCase { + + @Override + protected MissingQueryBuilder doCreateTestQueryBuilder() { + MissingQueryBuilder query = new MissingQueryBuilder(getRandomFieldName()); + if (randomBoolean()) { + query.nullValue(randomBoolean()); + } + if (randomBoolean()) { + query.existence(randomBoolean()); + } + // cannot set both to false + if ((query.nullValue() == false) && (query.existence() == false)) { + query.existence(!query.existence()); + } + return query; + } + + private String getRandomFieldName() { + if (randomBoolean()) { + return randomAsciiOfLengthBetween(1, 10); + } + switch (randomIntBetween(0, 3)) { + case 0: + return BOOLEAN_FIELD_NAME; + case 1: + return STRING_FIELD_NAME; + case 2: + return INT_FIELD_NAME; + case 3: + return DOUBLE_FIELD_NAME; + default: + throw new UnsupportedOperationException(); + } + } + + @Override + protected Query doCreateExpectedQuery(MissingQueryBuilder queryBuilder, QueryParseContext context) throws IOException { + final boolean existence = queryBuilder.existence(); + final boolean nullValue = queryBuilder.nullValue(); + String fieldPattern = queryBuilder.fieldPattern(); + + if (!existence && !nullValue) { + throw new QueryParsingException(context, "missing must have either existence, or null_value, or both set to true"); + } + + final FieldNamesFieldMapper.FieldNamesFieldType fieldNamesFieldType = (FieldNamesFieldMapper.FieldNamesFieldType) context.mapperService().fullName(FieldNamesFieldMapper.NAME); + if (fieldNamesFieldType == null) { + // can only happen when no types exist, so no docs exist either + return Queries.newMatchNoDocsQuery(); + } + + ObjectMapper objectMapper = context.getObjectMapper(fieldPattern); + if (objectMapper != null) { + // automatic make the object mapper pattern + fieldPattern = fieldPattern + ".*"; + } + + Collection fields = context.simpleMatchToIndexNames(fieldPattern); + if (fields.isEmpty()) { + if (existence) { + // if we ask for existence of fields, and we found none, then we should match on all + return Queries.newMatchAllQuery(); + } + return null; + } + + Query existenceFilter = null; + Query nullFilter = null; + + if (existence) { + BooleanQuery boolFilter = new BooleanQuery(); + for (String field : fields) { + MappedFieldType fieldType = context.fieldMapper(field); + Query filter = null; + if (fieldNamesFieldType.isEnabled()) { + final String f; + if (fieldType != null) { + f = fieldType.names().indexName(); + } else { + f = field; + } + filter = fieldNamesFieldType.termQuery(f, context); + } + // if _field_names are not indexed, we need to go the slow way + if (filter == null && fieldType != null) { + filter = fieldType.rangeQuery(null, null, true, true, context); + } + if (filter == null) { + filter = new TermRangeQuery(field, null, null, true, true); + } + boolFilter.add(filter, BooleanClause.Occur.SHOULD); + } + + existenceFilter = boolFilter; + existenceFilter = Queries.not(existenceFilter);; + } + + if (nullValue) { + for (String field : fields) { + MappedFieldType fieldType = context.fieldMapper(field); + if (fieldType != null) { + nullFilter = fieldType.nullValueQuery(); + } + } + } + + Query filter; + if (nullFilter != null) { + if (existenceFilter != null) { + BooleanQuery combined = new BooleanQuery(); + combined.add(existenceFilter, BooleanClause.Occur.SHOULD); + combined.add(nullFilter, BooleanClause.Occur.SHOULD); + // cache the not filter as well, so it will be faster + filter = combined; + } else { + filter = nullFilter; + } + } else { + filter = existenceFilter; + } + + if (filter == null) { + return null; + } + + return new ConstantScoreQuery(filter); + } + + @Test + public void testValidate() { + MissingQueryBuilder missingQueryBuilder = new MissingQueryBuilder(""); + assertThat(missingQueryBuilder.validate().validationErrors().size(), is(1)); + + missingQueryBuilder = new MissingQueryBuilder(null); + assertThat(missingQueryBuilder.validate().validationErrors().size(), is(1)); + + missingQueryBuilder = new MissingQueryBuilder("field").existence(false).nullValue(false); + assertThat(missingQueryBuilder.validate().validationErrors().size(), is(1)); + + missingQueryBuilder = new MissingQueryBuilder("field"); + assertNull(missingQueryBuilder.validate()); + } + + @Test(expected = QueryParsingException.class) + public void testBothNullValueAndExistenceFalse() throws IOException { + QueryParseContext context = createContext(); + context.setAllowUnmappedFields(true); + MissingQueryBuilder.newFilter(context, "field", false, false); + } +}