-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Fix match_only_text for keyword multi-fields with ignore_above #131314
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
Fix match_only_text for keyword multi-fields with ignore_above #131314
Conversation
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
| } | ||
| return storedFieldFetcher(parentField); | ||
| } else if (parent.hasDocValues()) { | ||
| var ifd = searchExecutionContext.getForField(parent, MappedFieldType.FielddataOperation.SEARCH); |
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.
It seems like there can be a similar problem here because doc values won't include ignored values? Not sure if that matters though.
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.
Shoot you're right. If the keyword field is store: false, doc_values: true, we don't error, but we completely omit the value from the match_only_text field results.
I'll work on a follow-up PR to address this.
💔 Backport failed
You can use sqren/backport to manually backport by running |
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
...per-extras/src/main/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapper.java
Show resolved
Hide resolved
| if (names.length == 1) { | ||
| return storedFields.get(names[0]); | ||
| } | ||
| return Arrays.stream(names).map(storedFields::get).filter(Objects::nonNull).flatMap(List::stream).toList(); |
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.
Nit: streams are not very efficient, it's better to avoid them for code that runs per document.
In #131314 we fixed match_only_text fields with ignore_above keyword multi-fields in the case that the keyword multi-field is stored. However, the issue is still present if the keyword field is not stored, but instead has doc values. This patch fixes that case.
In elastic#131314 we fixed match_only_text fields with ignore_above keyword multi-fields in the case that the keyword multi-field is stored. However, the issue is still present if the keyword field is not stored, but instead has doc values. This patch fixes that case.
In elastic#131314 we fixed match_only_text fields with ignore_above keyword multi-fields in the case that the keyword multi-field is stored. However, the issue is still present if the keyword field is not stored, but instead has doc values. This patch fixes that case.
In #131314 we fixed match_only_text fields with ignore_above keyword multi-fields in the case that the keyword multi-field is stored. However, the issue is still present if the keyword field is not stored, but instead has doc values. This patch fixes that case.
In #131314 we fixed match_only_text fields with ignore_above keyword multi-fields in the case that the keyword multi-field is stored. However, the issue is still present if the keyword field is not stored, but instead has doc values. This patch fixes that case.
In #129126, we stopped double-storing match_only_text fields when they are part of a multi-field, instead extracting the value when needed from the appropriate multi-field mapper.
This introduced an edge case related to ignore_above on keyword fields. If the associated multi-field mapper is a keyword mapper, and if that keyword mapper has ignore_above specified, and a document triggers the ignore_above case, then the original value will be stored in
<foo>._originalinstead of<foo>. In this case, the match_only_text mapper needs to look at the<foo>._originalstored field.Resolves #131298