From 45197d482078bd7ae95ba8067977acd5ad3718ad Mon Sep 17 00:00:00 2001
From: Jim Ferenczi <jimczi@apache.org>
Date: Tue, 21 Aug 2018 12:46:16 +0200
Subject: [PATCH 1/3] Change query field expansion
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
---
.../index/mapper/MappedFieldType.java | 9 ++-
.../index/search/QueryParserHelper.java | 71 ++++---------------
.../search/query/QueryStringIT.java | 4 +-
.../search/query/all-query-index.json | 8 +--
4 files changed, 28 insertions(+), 64 deletions(-)
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java b/server/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java
index 5f3f4a4de49d6..4a3fa852e7f7d 100644
--- a/server/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java
+++ b/server/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java
@@ -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;
@@ -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
+ */
+ // 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
diff --git a/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java b/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java
index df96ff87ec256..004424c50f73d 100644
--- a/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java
+++ b/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java
@@ -20,46 +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.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() {}
/**
@@ -85,22 +58,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) {
- DocumentMapper mapper = mapperService.documentMapper();
- if (mapper != null) {
- 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);
@@ -138,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,
@@ -154,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
@@ -177,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 (RuntimeException e) {
+ // other exceptions are parsing errors or not indexed fields: keep
+ }
}
fields.put(fieldName, weight);
}
diff --git a/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java b/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java
index 5caab8c9dfec6..a90e98a38eefc 100644
--- a/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java
+++ b/server/src/test/java/org/elasticsearch/search/query/QueryStringIT.java
@@ -430,8 +430,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();
diff --git a/server/src/test/resources/org/elasticsearch/search/query/all-query-index.json b/server/src/test/resources/org/elasticsearch/search/query/all-query-index.json
index abdc11928229f..9ab8995813e33 100644
--- a/server/src/test/resources/org/elasticsearch/search/query/all-query-index.json
+++ b/server/src/test/resources/org/elasticsearch/search/query/all-query-index.json
@@ -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"},
@@ -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"}
}
}
From cac2e94b6bc39f5fce1792803469d680906a2fa2 Mon Sep 17 00:00:00 2001
From: Jim Ferenczi <jimczi@apache.org>
Date: Tue, 21 Aug 2018 14:34:55 +0200
Subject: [PATCH 2/3] Catch IAE instead of RuntimeException
---
.../java/org/elasticsearch/index/search/QueryParserHelper.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java b/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java
index 004424c50f73d..97a07455534f6 100644
--- a/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java
+++ b/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java
@@ -143,7 +143,7 @@ public static Map<String, Float> resolveMappingField(QueryShardContext context,
} catch (QueryShardException |UnsupportedOperationException e) {
// field type is never searchable with term queries (eg. geo point): ignore
continue;
- } catch (RuntimeException e) {
+ } catch (IllegalArgumentException e) {
// other exceptions are parsing errors or not indexed fields: keep
}
}
From b38d605315c896ed47bbc0ddff6798a2152329d3 Mon Sep 17 00:00:00 2001
From: Jim Ferenczi <jimczi@apache.org>
Date: Tue, 21 Aug 2018 16:22:26 +0200
Subject: [PATCH 3/3] add ElasticsearchParseException
---
.../java/org/elasticsearch/index/search/QueryParserHelper.java | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java b/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java
index 97a07455534f6..d3bac583eac68 100644
--- a/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java
+++ b/server/src/main/java/org/elasticsearch/index/search/QueryParserHelper.java
@@ -19,6 +19,7 @@
package org.elasticsearch.index.search;
+import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.query.QueryShardContext;
@@ -143,7 +144,7 @@ public static Map<String, Float> resolveMappingField(QueryShardContext context,
} catch (QueryShardException |UnsupportedOperationException e) {
// field type is never searchable with term queries (eg. geo point): ignore
continue;
- } catch (IllegalArgumentException e) {
+ } catch (IllegalArgumentException |ElasticsearchParseException e) {
// other exceptions are parsing errors or not indexed fields: keep
}
}