Add "all fields" execution mode to simple_query_string query #21341

dakrone · 2016-11-04T16:17:19Z

This commit introduces a new execution mode for the
simple_query_string query, which is intended down the road to be a
replacement for the current _all field.

It now does auto-field-expansion and auto-leniency when the following criteria
are ALL met:

The _all field is disabled
No default_field has been set in the index settings
No fields are specified in the request

Additionally, a user can force the "all-like" execution by setting the
all_fields parameter to true.

When executing in all field mode, the simple_query_string query will
look at all the fields in the mapping that are not metafields and can be
searched, and automatically expand the list of fields that are going to
be queried.

Relates to #20925, which is the query_string version of this work.
This is basically the same behavior, but for the simple_query_string
query.

Relates to #19784

jimczi

Left two minors, LGTM otherwise

jimczi · 2016-11-07T09:37:24Z

core/src/test/java/org/elasticsearch/search/query/SimpleQueryStringIT.java

+        SearchResponse resp = client().prepareSearch("test").setQuery(
+                simpleQueryStringQuery("127.0.0.2 \"2015/09/02\"")
+                .field("f_ip") // Usually this would mean we wouldn't search "all" fields
+                .useAllFields(true)) // ... unless explicitly requested


Maybe just throw an exception in that case (like fields does) ?

Yes! I actually missed this and the exception throwing was only validating the xcontent parsing, so things like the transport client weren't tripping it. I've changed that to validate it always.

jimczi · 2016-11-07T09:49:51Z

core/src/test/java/org/elasticsearch/search/query/SimpleQueryStringIT.java

+        // suggest doesn't match
+        // geo_point doesn't match
+        // geo_shape doesn't match
+    }


I think you'll want to have a test mixing numbers and text ?

Sure, I've added another test for mixing the types (other tests in the suite do this as well)

jpountz · 2016-11-07T15:19:02Z

The fact that we need to implement this feature on multiple query parsers makes me wonder whether this could be done at the mapping level, ie the _all field type would check whether it is enabled, and return a disjunction over all existing (and supported) fields when it is disabled. Maybe this way we could make it transparent to query parsers? One issue I can think is that we would need to add a phraseQuery method to MappedFieldType so that fields could customize how phrase queries are handled (_all would need to return a disjunction over phrase queries for each matching field I think?).

clintongormley · 2016-11-07T15:40:08Z

@jpountz i thought of something similar, but do we really want this mode to be used in (eg) term queries?

dakrone · 2016-11-07T15:59:16Z

@jpountz when I talked to Clint about it (basically creating a "meta-field"), I think the all_fields parameter is better. We have to implement it on multiple query parsers, however, the implementation is slightly different for each, and there are only 3 that are really being targeted (query_string, simple_query_string, and multi_match). I think implementing it at the mapping level is going to be just as or more complex.

jpountz · 2016-11-07T16:40:44Z

do we really want this mode to be used in (eg) term queries?

Good question. We already do something similar with the _index field, which you can query with a term query even though it is not stored explicitly. If we really want to have a distinction, we could make sure to use the already existing MappedFieldType.queryStringTermQuery method in query parsers, and termQuery in term queries, which I suspect was the initial goal of having those two methods.

I think implementing it at the mapping level is going to be just as or more complex.

Fair enough, I just wanted to make sure we considered this option.

dakrone · 2016-11-07T22:58:29Z

retest this please

dakrone · 2016-11-08T16:10:12Z

retest this please

dakrone · 2016-11-08T16:17:57Z

retest this please but don't fail pulling the builds this time :-/

This commit introduces a new execution mode for the `simple_query_string` query, which is intended down the road to be a replacement for the current _all field. It now does auto-field-expansion and auto-leniency when the following criteria are ALL met: The _all field is disabled No default_field has been set in the index settings No fields are specified in the request Additionally, a user can force the "all-like" execution by setting the all_fields parameter to true. When executing in all field mode, the `simple_query_string` query will look at all the fields in the mapping that are not metafields and can be searched, and automatically expand the list of fields that are going to be queried. Relates to elastic#20925, which is the `query_string` version of this work. This is basically the same behavior, but for the `simple_query_string` query. Relates to elastic#19784

As part of elastic#20925 and elastic#21341 we added an "all-fields" mode to the `query_string` and `simple_query_string`. This would expand the query to all fields and automatically set `lenient` to true. However, we should still allow a user to override the `lenient` flag to whichever value they desire, should they add it in the request. This commit does that.

This change disables the _all meta field by default. Now that we have the "all-fields" method of query execution, we can save both indexing time and disk space by disabling it. _all can no longer be configured for indices created after 6.0. Relates to elastic#20925 and elastic#21341 Resolves elastic#19784

dakrone added review v6.0.0-alpha1 v5.1.1 labels Nov 4, 2016

jimczi reviewed Nov 7, 2016

View reviewed changes

dakrone force-pushed the sqs-all-field-mode branch from 34806a7 to a1b6bb5 Compare November 9, 2016 17:38

dakrone force-pushed the sqs-all-field-mode branch from a1b6bb5 to 7420fd0 Compare November 9, 2016 17:38

dakrone merged commit 7420fd0 into elastic:master Nov 9, 2016

clintongormley added the >feature label Nov 10, 2016

dakrone mentioned this pull request Nov 11, 2016

Allow overriding all-field leniency when lenient option is specified #21504

Merged

dakrone mentioned this pull request Dec 13, 2016

Disable _all by default, disallow configuring _all on 6.0+ indices #22144

Merged

dakrone deleted the sqs-all-field-mode branch January 23, 2017 17:23

clintongormley added :Search/Search Search-related issues that do not fall into other categories and removed :Query DSL labels Feb 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add "all fields" execution mode to simple_query_string query #21341

Add "all fields" execution mode to simple_query_string query #21341

dakrone commented Nov 4, 2016

jimczi left a comment

jimczi Nov 7, 2016

dakrone Nov 7, 2016

jimczi Nov 7, 2016

dakrone Nov 7, 2016

jpountz commented Nov 7, 2016

clintongormley commented Nov 7, 2016

dakrone commented Nov 7, 2016

jpountz commented Nov 7, 2016

dakrone commented Nov 7, 2016

dakrone commented Nov 8, 2016

dakrone commented Nov 8, 2016

Add "all fields" execution mode to simple_query_string query #21341

Add "all fields" execution mode to simple_query_string query #21341

Conversation

dakrone commented Nov 4, 2016

jimczi left a comment

Choose a reason for hiding this comment

jimczi Nov 7, 2016

Choose a reason for hiding this comment

dakrone Nov 7, 2016

Choose a reason for hiding this comment

jimczi Nov 7, 2016

Choose a reason for hiding this comment

dakrone Nov 7, 2016

Choose a reason for hiding this comment

jpountz commented Nov 7, 2016

clintongormley commented Nov 7, 2016

dakrone commented Nov 7, 2016

jpountz commented Nov 7, 2016

dakrone commented Nov 7, 2016

dakrone commented Nov 8, 2016

dakrone commented Nov 8, 2016