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

[7.7.0] Missing breaking change: null values for minimum_should_match #56812

Closed
jayaddison opened this issue May 15, 2020 · 6 comments · Fixed by #56817
Closed

[7.7.0] Missing breaking change: null values for minimum_should_match #56812

jayaddison opened this issue May 15, 2020 · 6 comments · Fixed by #56817
Assignees
Labels
>bug :Search/Search Search-related issues that do not fall into other categories

Comments

@jayaddison
Copy link

Elasticsearch version (bin/elasticsearch --version): 7.7.0

Plugins installed: []

JVM version (java -version): openjdk 14 2020-03-17

OS version (uname -a if on a Unix-like system): Linux basecamp 5.4.0-29-generic #33-Ubuntu SMP Wed Apr 29 14:32:27 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

Description of the problem including expected versus actual behavior:
During execution of a bool query with minimum_should_match value set to null, an x_content_parse_exception was raised:

[bool] failed to parse field [minimum_should_match]

Steps to reproduce:

  1. Perform a bool query against an ES 7.7.0 cluster:
curl -H 'Content-Type: application/json' localhost:9200/_search --data '{"query": {"bool": {"minimum_should_match": null}}}'
  1. Encounter the error response:
{"error":{"root_cause":[{"type":"x_content_parse_exception","reason":"[1:45] [bool] failed to parse field [minimum_should_match]"}],"type":"x_content_parse_exception","reason":"[1:45] [bool] failed to parse field [minimum_should_match]","caused_by":{"type":"illegal_state_exception","reason":"Can't get text on a VALUE_NULL at 1:45"}},"status":400}

Expected behaviour:
This query was valid in ES 7.6.2, and doesn't appear in the 'breaking changes' for 7.7.0 (ref), so this failure was unexpected.

  1. Perform a bool query against an ES 7.6.2 cluster:
curl -H 'Content-Type: application/json' localhost:9200/_search --data '{"query": {"bool": {"minimum_should_match": null}}}'
  1. Encounter a valid search response:
{"took":0,"timed_out":false,"_shards":{"total":0,"successful":0,"skipped":0,"failed":0},"hits":{"total":{"value":0,"relation":"eq"},"max_score":0.0,"hits":[]}}
@jayaddison jayaddison added >bug needs:triage Requires assignment of a team area label labels May 15, 2020
@cbuescher cbuescher added the :Search/Search Search-related issues that do not fall into other categories label May 15, 2020
@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 cbuescher removed Team:Search Meta label for search team needs:triage Requires assignment of a team area label labels May 15, 2020
@cbuescher
Copy link
Member

@jayaddison thanks for raising this, this is likely an unintended side effect of #52880. If we accepted null here and treated it like the default we should continue to do so on at least 7.x versions.

@cbuescher cbuescher self-assigned this May 15, 2020
cbuescher pushed a commit to cbuescher/elasticsearch that referenced this issue May 15, 2020
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
@jayaddison
Copy link
Author

Brilliant, and thanks for responding to this so quickly, @cbuescher. I've been able to roll-forward my application's behaviour to cope with the change in null handling, but I imagine others may still appreciate the fix.

@bcbee
Copy link

bcbee commented May 26, 2020

Filed ES support case 00549044 for similar issue with must/should/must_not/filter. Very easy fix in our app but broke some stuff in production.

cbuescher pushed a commit that referenced this issue 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 issue 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 issue 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 issue 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
@lanerjo
Copy link

lanerjo commented Sep 4, 2020

How is this closed? This issue is still present in 7.8.1

@cbuescher
Copy link
Member

This issue is still present in 7.8.1

I just checked this on a local test instance and on cloud in 7.8.1 and the reported issue is fixed there. Can you make sure that you are not running any older nodes in your cluster? If you are still having issues I'd like to point you to to the support forums over at https://discuss.elastic.co to do further digging since I cannot reproduce this issue here. Many thanks.

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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants