Fix match_only_text bugs if defined as multi-field #130188
Merged
Conversation
This file contains hidden or 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
Bugs starting to occur when elastic#129126 was merged. Closes elastic#129737
martijnvg
added
>non-issue
:StorageEngine/Mapping
|
Pinging @elastic/es-storage-engine (Team:StorageEngine) |
kkrik-es
reviewed
Jun 28, 2025
| if (searchExecutionContext.isSourceSynthetic()) { | ||
| if (searchExecutionContext.isSourceSynthetic() && withinMultiField == false && hasCompatibleMultiFields == false) { | ||
| String name = storedFieldNameForSyntheticSource(); | ||
| // TODO: go the parent field and load either via stored fields or doc values the values instead synthesizing complete source |
There was a problem hiding this comment.
Suggested change
| // TODO: go the parent field and load either via stored fields or doc values the values instead synthesizing complete source | |
| // TODO: go the parent field and load either via stored fields or doc values, instead of synthesizing the complete source. |
kkrik-es
reviewed
Jun 28, 2025
| if (searchExecutionContext.isSourceSynthetic() && withinMultiField == false && hasCompatibleMultiFields == false) { | ||
| String name = storedFieldNameForSyntheticSource(); | ||
| // TODO: go the parent field and load either via stored fields or doc values the values instead synthesizing complete source | ||
| // (in case of synthetic source and if this field is a multi field, then it will not have a stored field.) |
There was a problem hiding this comment.
Suggested change
| // (in case of synthetic source and if this field is a multi field, then it will not have a stored field.) | |
| // In case of synthetic source and if this field is a multi field, then it will not have a stored field. |
kkrik-es
reviewed
Jun 28, 2025
| ); | ||
| } | ||
| if (searchExecutionContext.isSourceSynthetic()) { | ||
| if (searchExecutionContext.isSourceSynthetic() && withinMultiField == false && hasCompatibleMultiFields == false) { |
There was a problem hiding this comment.
Nit: these two variables are only used here? If so, replace them with one?
There was a problem hiding this comment.
This is actually for the follow up. When there is a keyword multi field then doc values or stored field of the multi field is used, otherwise if stored field / doc values of parent keyword field is used.
kkrik-es
reviewed
Jun 28, 2025
| if (searchExecutionContext.isSourceSynthetic()) { | ||
| if (searchExecutionContext.isSourceSynthetic() && withinMultiField == false && hasCompatibleMultiFields == false) { | ||
| String name = storedFieldNameForSyntheticSource(); | ||
| // TODO: go the parent field and load either via stored fields or doc values the values instead synthesizing complete source |
There was a problem hiding this comment.
Does this comment belong to the next block that invokes SourceProvider?
There was a problem hiding this comment.
Yes, the values would be loaded via source provider, which is slow. I will fix this.
kkrik-es
reviewed
Jun 28, 2025
| } | ||
| ); | ||
| } else { | ||
| var kwd = TextFieldMapper.SyntheticSourceHelper.getKeywordFieldMapperForSyntheticSource(this); |
There was a problem hiding this comment.
Is keyword the only multifield combination with match_only_text?
There was a problem hiding this comment.
Yes, in other cases the match_only_text field will be stored.
kkrik-es
approved these changes
Jun 28, 2025
…tored field or doc values of parent keyword field or stored field or doc values of multi field.
martijnvg
added a commit
to martijnvg/elasticsearch
that referenced
this pull request
Jun 30, 2025
* Fix match_only_text bugs if defined as multi-field Bugs starting to occur when elastic#129126 was merged. Closes elastic#129737
💔 Backport failed
You can use sqren/backport to manually backport by running |
elasticsearchmachine
pushed a commit
that referenced
this pull request
Jun 30, 2025
martijnvg
added a commit
to martijnvg/elasticsearch
that referenced
this pull request
Jun 30, 2025
Backporting elastic#130188 to 8.19 branch. Bugs starting to occur when elastic#129126 was merged. Closes elastic#129737
elasticsearchmachine
pushed a commit
that referenced
this pull request
Jun 30, 2025
mridula-s109
pushed a commit
to mridula-s109/elasticsearch
that referenced
this pull request
Jul 3, 2025
* Fix match_only_text bugs if defined as multi-field Bugs starting to occur when elastic#129126 was merged. Closes elastic#129737
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Bugs starting to occur when #129126 was merged.
Closes #129737
This is a bug, but marking as non-issue. Given that it has not been released in a stateful release.