Skip to content

Ensure fetch fields aren't dropped when rewriting search. #61383

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

Merged
merged 3 commits into from
Aug 20, 2020

Conversation

jtibshirani
Copy link
Contributor

Previously we didn't retain the requested fields when performing a shallow copy
of the search source. This meant that when a search was rewritten, we could drop
the requested fields and fail to return them in the response.

Previously we didn't retain the requested fields when performing a shallow copy
of the search source. This meant that when a search was rewritten, we could drop
the requested fields and fail to return them in the response.
@jtibshirani jtibshirani added >non-issue :Search/Search Search-related issues that do not fall into other categories v8.0.0 v7.10.0 labels Aug 20, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/Search)

@elasticmachine elasticmachine added the Team:Search Meta label for search team label Aug 20, 2020
@jtibshirani
Copy link
Contributor Author

I've marked this as 'non-issue' because it wasn't released. For context, @astefan discovered this bug through the SQL test suites.

Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks for adding the shallow copy test.

public void testShallowCopy() {
SearchSourceBuilder original = createSearchSourceBuilder();
SearchSourceBuilder copy = original.shallowCopy();
assertEquals(original, copy);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++, maybe run few iterations to catch more cases ?

@jtibshirani
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/packaging-sample-windows

@jtibshirani
Copy link
Contributor Author

Thanks @jimczi for the review.

@jtibshirani jtibshirani merged commit 468e58b into elastic:master Aug 20, 2020
@jtibshirani jtibshirani deleted the fields-search-rewrite branch August 20, 2020 21:11
jtibshirani added a commit to jtibshirani/elasticsearch that referenced this pull request Aug 20, 2020
)

Previously we didn't retain the requested fields when performing a shallow copy
of the search source. This meant that when a search was rewritten, we could drop
the requested fields and fail to return them in the response.
jtibshirani added a commit that referenced this pull request Aug 20, 2020
The skip can be removed now that #61383 is backported.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>non-issue :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team v7.10.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants