Skip to content

Commit

Permalink
Change query field expansion (#33020)
Browse files Browse the repository at this point in the history
This commit changes the query field expansion for query parsers
to not rely on an hardcoded list of field types. Instead we rely on
the type of exception that is thrown by MappedFieldType#termQuery to
include/exclude an expanded field.

Supersedes #31655

Closes #31798
  • Loading branch information
jimczi committed Aug 23, 2018
1 parent 9a3d341 commit 500d533
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 63 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.apache.lucene.search.TermInSetQuery;
import org.apache.lucene.search.TermQuery;
import org.apache.lucene.util.BytesRef;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.geo.ShapeRelation;
import org.elasticsearch.common.joda.DateMathParser;
Expand Down Expand Up @@ -336,7 +337,13 @@ public boolean isAggregatable() {
/** Generates a query that will only match documents that contain the given value.
* The default implementation returns a {@link TermQuery} over the value bytes,
* boosted by {@link #boost()}.
* @throws IllegalArgumentException if {@code value} cannot be converted to the expected data type */
* @throws IllegalArgumentException if {@code value} cannot be converted to the expected data type or if the field is not searchable
* due to the way it is configured (eg. not indexed)
* @throws ElasticsearchParseException if {@code value} cannot be converted to the expected data type
* @throws UnsupportedOperationException if the field is not searchable regardless of options
* @throws QueryShardException if the field is not searchable regardless of options
*/
// TODO: Standardize exception types
public abstract Query termQuery(Object value, @Nullable QueryShardContext context);

/** Build a constant-scoring query that matches all values. The default implementation uses a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,47 +19,21 @@

package org.elasticsearch.index.search;

import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.index.mapper.DateFieldMapper;
import org.elasticsearch.index.mapper.DocumentMapper;
import org.elasticsearch.index.mapper.FieldMapper;
import org.elasticsearch.index.mapper.IpFieldMapper;
import org.elasticsearch.index.mapper.KeywordFieldMapper;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.mapper.Mapper;
import org.elasticsearch.index.mapper.MapperService;
import org.elasticsearch.index.mapper.MetadataFieldMapper;
import org.elasticsearch.index.mapper.NumberFieldMapper;
import org.elasticsearch.index.mapper.TextFieldMapper;
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.index.query.QueryShardException;

import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

/**
* Helpers to extract and expand field names and boosts
*/
public final class QueryParserHelper {
// Mapping types the "all-ish" query can be executed against
// TODO: Fix the API so that we don't need a hardcoded list of types
private static final Set<String> ALLOWED_QUERY_MAPPER_TYPES;

static {
ALLOWED_QUERY_MAPPER_TYPES = new HashSet<>();
ALLOWED_QUERY_MAPPER_TYPES.add(DateFieldMapper.CONTENT_TYPE);
ALLOWED_QUERY_MAPPER_TYPES.add(IpFieldMapper.CONTENT_TYPE);
ALLOWED_QUERY_MAPPER_TYPES.add(KeywordFieldMapper.CONTENT_TYPE);
for (NumberFieldMapper.NumberType nt : NumberFieldMapper.NumberType.values()) {
ALLOWED_QUERY_MAPPER_TYPES.add(nt.typeName());
}
ALLOWED_QUERY_MAPPER_TYPES.add("scaled_float");
ALLOWED_QUERY_MAPPER_TYPES.add(TextFieldMapper.CONTENT_TYPE);
}

private QueryParserHelper() {}

/**
Expand All @@ -85,21 +59,6 @@ public static Map<String, Float> parseFieldsAndWeights(List<String> fields) {
return fieldsAndWeights;
}

/**
* Get a {@link FieldMapper} associated with a field name or null.
* @param mapperService The mapper service where to find the mapping.
* @param field The field name to search.
*/
public static Mapper getFieldMapper(MapperService mapperService, String field) {
for (DocumentMapper mapper : mapperService.docMappers(true)) {
Mapper fieldMapper = mapper.mappers().getMapper(field);
if (fieldMapper != null) {
return fieldMapper;
}
}
return null;
}

public static Map<String, Float> resolveMappingFields(QueryShardContext context,
Map<String, Float> fieldsAndWeights) {
return resolveMappingFields(context, fieldsAndWeights, null);
Expand Down Expand Up @@ -136,8 +95,7 @@ public static Map<String, Float> resolveMappingFields(QueryShardContext context,
* @param fieldOrPattern The field name or the pattern to resolve
* @param weight The weight for the field
* @param acceptAllTypes Whether all field type should be added when a pattern is expanded.
* If false, only {@link #ALLOWED_QUERY_MAPPER_TYPES} are accepted and other field types
* are discarded from the query.
* If false, only searchable field types are added.
* @param acceptMetadataField Whether metadata fields should be added when a pattern is expanded.
*/
public static Map<String, Float> resolveMappingField(QueryShardContext context, String fieldOrPattern, float weight,
Expand All @@ -152,8 +110,7 @@ public static Map<String, Float> resolveMappingField(QueryShardContext context,
* @param fieldOrPattern The field name or the pattern to resolve
* @param weight The weight for the field
* @param acceptAllTypes Whether all field type should be added when a pattern is expanded.
* If false, only {@link #ALLOWED_QUERY_MAPPER_TYPES} are accepted and other field types
* are discarded from the query.
* If false, only searchable field types are added.
* @param acceptMetadataField Whether metadata fields should be added when a pattern is expanded.
* @param fieldSuffix The suffix name to add to the expanded field names if a mapping exists for that name.
* The original name of the field is kept if adding the suffix to the field name does not point to a valid field
Expand All @@ -175,18 +132,20 @@ public static Map<String, Float> resolveMappingField(QueryShardContext context,
continue;
}

// Ignore fields that are not in the allowed mapper types. Some
// types do not support term queries, and thus we cannot generate
// a special query for them.
String mappingType = fieldType.typeName();
if (acceptAllTypes == false && ALLOWED_QUERY_MAPPER_TYPES.contains(mappingType) == false) {
if (acceptMetadataField == false && fieldType.name().startsWith("_")) {
// Ignore metadata fields
continue;
}

// Ignore metadata fields.
Mapper mapper = getFieldMapper(context.getMapperService(), fieldName);
if (acceptMetadataField == false && mapper instanceof MetadataFieldMapper) {
continue;
if (acceptAllTypes == false) {
try {
fieldType.termQuery("", context);
} catch (QueryShardException |UnsupportedOperationException e) {
// field type is never searchable with term queries (eg. geo point): ignore
continue;
} catch (IllegalArgumentException |ElasticsearchParseException e) {
// other exceptions are parsing errors or not indexed fields: keep
}
}
fields.put(fieldName, weight);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -409,8 +409,8 @@ public void testFieldAliasOnDisallowedFieldType() throws Exception {
indexRequests.add(client().prepareIndex("test", "_doc", "1").setSource("f3", "text", "f2", "one"));
indexRandom(true, false, indexRequests);

// The wildcard field matches aliases for both a text and boolean field.
// By default, the boolean field should be ignored when building the query.
// The wildcard field matches aliases for both a text and geo_point field.
// By default, the geo_point field should be ignored when building the query.
SearchResponse response = client().prepareSearch("test")
.setQuery(queryStringQuery("text").field("f*_alias"))
.execute().actionGet();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@
"format": "yyyy/MM/dd||epoch_millis"
},
"f_bool": {"type": "boolean"},
"f_bool_alias": {
"type": "alias",
"path": "f_bool"
},
"f_byte": {"type": "byte"},
"f_short": {"type": "short"},
"f_int": {"type": "integer"},
Expand All @@ -60,6 +56,10 @@
"f_binary": {"type": "binary"},
"f_suggest": {"type": "completion"},
"f_geop": {"type": "geo_point"},
"f_geop_alias": {
"type": "alias",
"path": "f_geop"
},
"f_geos": {"type": "geo_shape"}
}
}
Expand Down

0 comments on commit 500d533

Please sign in to comment.