Skip to content

Fix simple_query_string on invalid input #28219

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 5 commits into from
Jan 18, 2018

Conversation

jimczi
Copy link
Contributor

@jimczi jimczi commented Jan 15, 2018

This change converts any exception that occurs during the parsing of
a simple_query_string to a match_no_docs query (instead of a null query)
when leniency is activated.

Closes #28204

This change converts any exception that occurs during the parsing of
a simple_query_string to a match_no_docs query (instead of a null query)
when leniency is activated.

Closes elastic#28204
@jimczi jimczi added >bug :Search/Search Search-related issues that do not fall into other categories v7.0.0 v6.2.0 labels Jan 15, 2018
Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -90,7 +91,7 @@ private Analyzer getAnalyzer(MappedFieldType ft) {
*/
private Query rethrowUnlessLenient(RuntimeException e) {
if (settings.lenient()) {
return null;
return Queries.newMatchNoDocsQuery("failed query, caused by " + e.getMessage());
Copy link
Member

@cbuescher cbuescher Jan 15, 2018

Choose a reason for hiding this comment

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

nit: can you update the comment to reflect that we return MatchNoDocs instead of null now?

@cbuescher
Copy link
Member

@jimczi also looked at the additional commit, makes sense. I left one suggestion regarding changes in the comment, other than that this looks good.

Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

Great, good to go once CI is green I think.

@jimczi jimczi merged commit c38c12e into elastic:master Jan 18, 2018
@jimczi jimczi deleted the bugs/lenient_simple_query_string branch January 18, 2018 09:49
jimczi added a commit that referenced this pull request Jan 18, 2018
This change converts any exception that occurs during the parsing of
a simple_query_string to a match_no_docs query (instead of a null query)
when leniency is activated.

Closes #28204
martijnvg added a commit that referenced this pull request Jan 22, 2018
* es/master: (38 commits)
  Build: Add pom generation to meta plugins (#28321)
  Add 6.3 version constant to master
  Minor improvements to translog docs (#28237)
  [Docs] Remove typo in painless-getting-started.asciidoc
  Build: Fix meta plugin usage in integ test clusters (#28307)
  Painless: Add spi jar that will be published for extending whitelists (#28302)
  mistyping in one of the highlighting examples comment -> content (#28139)
  Documents applicability of term query to range type (#28166)
  Build: Omit dependency licenses check for elasticsearch deps (#28304)
  Clean up commits when global checkpoint advanced (#28140)
  Implement socket and server ChannelContexts (#28275)
  Plugins: Fix meta plugins to install bundled plugins with their real name (#28285)
  Build: Fix meta plugin integ test installation (#28286)
  Modify Abstract transport tests to use impls (#28270)
  Fork Groovy compiler onto compile Java home
  [Docs] Update tophits-aggregation.asciidoc (#28273)
  Docs: match between snippet to its description (#28296)
  [TEST] fix RequestTests#testSearch in case search source is not set
  REST high-level client: remove index suffix from indices client method names (#28263)
  Fix simple_query_string on invalid input (#28219)
  ...
martijnvg added a commit that referenced this pull request Jan 22, 2018
* es/6.x: (38 commits)
  Build: Add pom generation to meta plugins (#28321)
  Add 6.3.0 version constant
  Minor improvements to translog docs (#28237)
  [Docs] Remove typo in painless-getting-started.asciidoc
  Build: Fix meta plugin usage in integ test clusters (#28307)
  Painless: Add spi jar that will be published for extending whitelists (#28302)
  mistyping in one of the highlighting examples comment -> content (#28139)
  Documents applicability of term query to range type (#28166)
  Build: Omit dependency licenses check for elasticsearch deps (#28304)
  Fix packaging test when checking for cgroups v2
  Plugins: Fix meta plugins to install bundled plugins with their real name (#28285)
  Build: Fix meta plugin integ test installation (#28286)
  Fork Groovy compiler onto compile Java home
  [Docs] Update tophits-aggregation.asciidoc (#28273)
  Skip restart upgrades for buggy cgroup2 handling
  Skip packaging upgrade test when cgroups v2 enabled
  Docs: match between snippet to its description (#28296)
  [TEST] fix RequestTests#testSearch in case search source is not set
  REST high-level client: remove index suffix from indices client method names (#28263)
  Fix simple_query_string on invalid input (#28219)
  ...
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
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 v6.2.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants