Fix index prefixes to work with span_multi #31066
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Text fields that use `index_prefixes` can rewrite `prefix` queries into `term` queries internally. This commit fix the handling of this rewriting in the `span_multi` query. This change also copies the index options of the text field into the prefix field in order to be able to run positional queries. This is mandatory for `span_multi` to work but this could also be useful to optimize `match_phrase_prefix` queries in a follow up. Note that this change can only be done on indices created after 6.3 since we set the index options to doc only in this version. Fixes elastic#31056
jimczi
added
>bug
:Search/Search
|
Pinging @elastic/es-search-aggs |
jpountz
approved these changes
Jun 4, 2018
| if (subQuery instanceof BoostQuery) { | ||
| if (subQuery instanceof ConstantScoreQuery) { | ||
| subQuery = ((ConstantScoreQuery) subQuery).getQuery(); | ||
| } else if (subQuery instanceof BoostQuery) { |
There was a problem hiding this comment.
should we unwrap recursively to be more resilient to changes to the way queries are built?
| */ | ||
| if (multiTermQueryBuilder.getClass() != PrefixQueryBuilder.class) { | ||
| throw new UnsupportedOperationException("unsupported inner query, should be " | ||
| + MultiTermQuery.class.getName() + " but was " + subQuery.getClass().getName()); |
There was a problem hiding this comment.
let's include the class of multiTermQueryBuilder to the error message as well?
| // Copy the index options of the main field to allow phrase queries on | ||
| // the prefix field. | ||
| if (context.indexCreatedVersion().onOrAfter(Version.V_7_0_0_alpha1)) { | ||
| prefixFieldType.setIndexOptions(fieldType.indexOptions()); |
There was a problem hiding this comment.
should we map DOCS_AND_FREQS TO DOCS?
| if (fieldType.indexOptions() == IndexOptions.DOCS_AND_FREQS_AND_POSITIONS_AND_OFFSETS) { | ||
| // Copy the index options of the main field to allow phrase queries on | ||
| // the prefix field. | ||
| if (context.indexCreatedVersion().onOrAfter(Version.V_7_0_0_alpha1)) { |
There was a problem hiding this comment.
I think this can already be set to 6_4 before the backport, can't it?
romseygeek
approved these changes
Jun 4, 2018
| @@ -339,7 +344,6 @@ public Query existsQuery(QueryShardContext context) { | |||
| PrefixFieldType(String name, int minChars, int maxChars) { | |||
| setTokenized(true); | |||
| setOmitNorms(true); | |||
| setIndexOptions(IndexOptions.DOCS); | |||
There was a problem hiding this comment.
I think we still need this? What happens with a 6.3 index that doesn't store offsets?
jimczi
added a commit
that referenced
this pull request
Jun 5, 2018
* Fix index prefixes to work with span_multi Text fields that use `index_prefixes` can rewrite `prefix` queries into `term` queries internally. This commit fix the handling of this rewriting in the `span_multi` query. This change also copies the index options of the text field into the prefix field in order to be able to run positional queries. This is mandatory for `span_multi` to work but this could also be useful to optimize `match_phrase_prefix` queries in a follow up. Note that this change can only be done on indices created after 6.3 since we set the index options to doc only in this version. Fixes #31056
martijnvg
added a commit
that referenced
this pull request
Jun 5, 2018
* es/6.x: Fix compil after backport Fix index prefixes to work with span_multi (#31066) Delete superfluous UpdateRequest test Disable MultiMatchQueryBuilderTests. Disable UpdateRequestTest#testUnknownFieldParsing. Update get.asciidoc (#31084) When the 'strict' mapping option is enabled, make sure to validate split_queries_on_whitespace. Transport client: Don't validate node in handshake (#30737) (#31080) In the internal highlighter APIs, use the field type as opposed to the mapper. (#31039) [DOCS] Removes redundant authorization pages Upgrade to Lucene-7.4.0-snapshot-0a7c3f462f (#31073)
jimczi
added a commit
that referenced
this pull request
Jun 5, 2018
multi_span prefix queries do not work with the prefix field before 6.4. Relates #31066
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Text fields that use
index_prefixescan rewriteprefixqueries intotermqueries internally. This commit fix the handling of this rewritingin the
span_multiquery.This change also copies the index options of the text field into the
prefix field in order to be able to run positional queries. This is mandatory
for
span_multito work but this could also be useful to optimizematch_phrase_prefixqueries in a follow up. Note that this change can only be done on indices created
after 6.3 since we set the index options to doc only in this version.
Fixes #31056