Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change the way that fields are selected for inclusion in all queries. #31655

Closed
wants to merge 1 commit into from
Closed

Change the way that fields are selected for inclusion in all queries. #31655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion server/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java
View file
Open in desktop
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 @@ -314,7 +315,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
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last 2 javadoc "throws" descriptions give identical reasons?

// 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
71 changes: 16 additions & 55 deletions server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,43 +20,19 @@
package org.elasticsearch.index.search;

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.MapperService;
import org.elasticsearch.index.mapper.MetadataFieldMapper;
import org.elasticsearch.index.mapper.NumberFieldMapper;
import org.elasticsearch.index.mapper.TextFieldMapper;
import org.elasticsearch.index.mapper.MappedFieldType;
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 @@ -83,22 +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 FieldMapper getFieldMapper(MapperService mapperService, String field) {
DocumentMapper mapper = mapperService.documentMapper();
if (mapper != null) {
FieldMapper 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 +96,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 +111,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 @@ -167,22 +125,25 @@ public static Map<String, Float> resolveMappingField(QueryShardContext context,
if (fieldSuffix != null && context.fieldMapper(fieldName + fieldSuffix) != null) {
fieldName = fieldName + fieldSuffix;
}
FieldMapper mapper = getFieldMapper(context.getMapperService(), fieldName);
if (mapper == null) {
MappedFieldType ft = context.fieldMapper(fieldName);
if (ft == null) {
// Unmapped fields are not ignored
fields.put(fieldOrPattern, weight);
continue;
}
if (acceptMetadataField == false && mapper instanceof MetadataFieldMapper) {
if (acceptMetadataField == false && ft.name().startsWith("_")) {
// Ignore metadata fields
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this break BWC for some users? Underscored fieldnames were always by convention "ours" but are we now being stronger on this (either with docs or enforcing naming conventions?)

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 = mapper.fieldType().typeName();
if (acceptAllTypes == false && ALLOWED_QUERY_MAPPER_TYPES.contains(mappingType) == false) {
continue;
if (acceptAllTypes == false) {
try {
ft.termQuery("", context);
} catch (QueryShardException|UnsupportedOperationException e) {
// field type is never searchable with term queries (eg. geo point): ignore
continue;
} catch (RuntimeException e) {
// other exceptions are parsing errors or not indexed fields: keep
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a similar capability-testing concern with field types that support phrase queries?

}
fields.put(fieldName, weight);
}
Expand Down