-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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
SourceValueFetcher should check all possible source fields #65375
Conversation
Pinging @elastic/es-search (Team: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.
Thanks for catching this important bug! I made a note about moving the test, but after that's addressed it looks ready to merge.
@@ -635,4 +642,34 @@ public void testCopyFromMultiField() { | |||
assertThat(e.getMessage(), | |||
Matchers.containsString("[copy_to] may not be used to copy from a multi-field: [field.bar]")); | |||
} | |||
|
|||
public void testCopyToValueFetching() throws IOException { |
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.
We could move this to FieldFetcherTests
, so all field retrieval tests are in one place. It already has a case around copy_to
.
@elasticmachine update branch |
This commit fixes a bug where SourceValueFetcher was returning an empty set of values if any of its source fields were empty; instead, we skip over the empty value and continue collecting from all other source fields.
This commit fixes a bug where SourceValueFetcher was returning
an empty set of values if any of its source fields were empty; instead,
we skip over the empty value and continue collecting from all other
source fields.