-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Search - make wildcard field use constant scoring queries for wildcard queries #70452
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.
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
Conversation
|
Pinging @elastic/es-search (Team:Search) |
|
Thanks to @jimczi for discovering the issue revealed as part of adding ConstantScoreQuery - there was a caching issue with AutomatonQueryOnBinaryDV objects due to not implementing hashcode/equals correctly for case sensitivity options. This PR also now includes a fix for that and related test addition. |
jimczi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some comments.
...wildcard/src/main/java/org/elasticsearch/xpack/wildcard/mapper/AutomatonQueryOnBinaryDv.java
Outdated
Show resolved
Hide resolved
...wildcard/src/test/java/org/elasticsearch/xpack/wildcard/mapper/WildcardFieldMapperTests.java
Outdated
Show resolved
Hide resolved
...ugin/wildcard/src/main/java/org/elasticsearch/xpack/wildcard/mapper/WildcardFieldMapper.java
Outdated
Show resolved
Hide resolved
|
One thing I wasn't sure on was when the AutomatonQueryOnBinaryDV should convert the Automaton to a ByteRunAutomaton. A ByteRunAutomaton looks to use a lot more memory than the source Automaton so might be expensive memory-wise to hold as a field when used as a cache key. The alternative of converting Automaton to ByteRunAutomaton every time we want to use it might slow down comparisons in hash look-ups. I've gone with the latter but not sure if that's the better trade-off. |
3c79796 to
7b5c86c
Compare
jimczi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left one comment regarding the building of the BytesRunAutomaton. LGTM otherwise.
...wildcard/src/main/java/org/elasticsearch/xpack/wildcard/mapper/AutomatonQueryOnBinaryDv.java
Outdated
Show resolved
Hide resolved
…. Add a note about ignoring rewrite parameters on wildcard queries. Closes elastic#69604
… caching bug revealed by change to ConstantScore wrapper
…ing a ByteRunAutomaton because Automaton doesn’t honour hashcode/equals.
…d queries and caching fix (elastic#70452) * Make wildcard field use constant scoring queries for wildcard queries. Add a note about ignoring rewrite parameters on wildcard queries. Also fixes caching issue where case sensitive and case insensitive results were cached as the same Closes elastic#69604
…d queries and caching fix (#70452) (#71043) Backport of 2f9c731 * Make wildcard field use constant scoring queries for wildcard queries. Add a note about ignoring rewrite parameters on wildcard queries. Also fixes caching issue where case sensitive and case insensitive results were cached as the same Closes #69604
Wrap the wildcard queries produced by the wildcard field as a constant scoring query because there is no sensible index to use for scoring terms with their frequencies.
Added a note about ignoring rewrite parameters on wildcard queries.
Also includes fix where case insensitive/sensitive wildcard queries cached on same key. Run WIldcardFieldMapperTest with seed
-Dtests.seed=722EAA9077BDA6AEto reveal the issue.Closes #69604