Skip to content
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 bool query behaviour on null value #56817

Merged
merged 3 commits into from
May 26, 2020
Merged

Conversation

cbuescher
Copy link
Member

@cbuescher cbuescher commented May 15, 2020

Until 7.7 we used to ignore null values for minimum_should_match in bool
queries. An internal refactoring has accidentally changed this so now we get a parsing error.
While null should not be a common value here, we should restore the old behaviour
at least on the 7.x lines.

Closes #56812

Until 7.7 we used to ignore `null` values for `minimum_should_match` in `bool`
queries. An internal refactoring has changed this so now we get a parsing error.
While `null` should not a common value here, we should restore the old behaviour
at least on the 7.x lines.

Closes elastic#56812
@cbuescher cbuescher added >bug :Search/Search Search-related issues that do not fall into other categories v8.0.0 v7.7.1 v7.8.1 v7.9.0 labels May 15, 2020
@cbuescher cbuescher requested a review from romseygeek May 15, 2020 13:04
@elasticmachine
Copy link
Collaborator

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

@elasticmachine elasticmachine added the Team:Search Meta label for search team label May 15, 2020
@cbuescher
Copy link
Member Author

fwiw I'm okay if we want to error on this in 8.0, in that case I'll leave a comment in the change notes but would port this back to open 7 branches. Also think accepting null like we used to is a fine option as well.

@cbuescher
Copy link
Member Author

@romseygeek it was also just brought up on the issue that before 7.7 also must, most_not, should and filter parameters were allowed to be null (no array) which was basically treated as a no-op. This seems to have been broken as well with the parser changes. I added a commit to adress that as well, again also happy to break this old behaviour in master but we should backport this to the 7 branches at least I think.

Copy link
Contributor

@romseygeek romseygeek left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @cbuescher

@cbuescher cbuescher merged commit 1211699 into elastic:master May 26, 2020
cbuescher pushed a commit that referenced this pull request May 26, 2020
Until 7.7 we used to ignore `null` values for `bool`queries `minimum_should_match`,
parameters and also for the `must`,  `must_not`, `should` and `filter` clauses.
An internal refactoring has changed this so now we get a parsing error. While `null` 
should not a common value here, we should restore the old behaviour for bwc for now.

Closes #56812
cbuescher pushed a commit that referenced this pull request May 26, 2020
Until 7.7 we used to ignore `null` values for `bool`queries `minimum_should_match`,
parameters and also for the `must`,  `must_not`, `should` and `filter` clauses.
An internal refactoring has changed this so now we get a parsing error. While `null` 
should not a common value here, we should restore the old behaviour for bwc for now.

Closes #56812
cbuescher pushed a commit that referenced this pull request May 26, 2020
Until 7.7 we used to ignore `null` values for `bool`queries `minimum_should_match`,
parameters and also for the `must`,  `must_not`, `should` and `filter` clauses.
An internal refactoring has changed this so now we get a parsing error. While `null` 
should not a common value here, we should restore the old behaviour for bwc for now.

Closes #56812
@cbuescher cbuescher changed the title Fix minimum_should_match behaviour on null value Fix bool query behaviour on null value May 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team v7.7.1 v7.8.1 v7.9.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[7.7.0] Missing breaking change: null values for minimum_should_match
4 participants