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

Fixed quote_field_suffix in query_string #29332

Merged
merged 4 commits into from
Apr 4, 2018

Conversation

jimczi
Copy link
Contributor

@jimczi jimczi commented Apr 2, 2018

This change fixes the handling of the quote_field_suffix option on query_string
query. The expansion was not applied to default fields query.

Closes #29324

This change fixes the handling of the `quote_field_suffix` option on `query_string`
 query. The expansion was not applied to default fields query.

Closes elastic#29324
@jimczi jimczi added >bug :Search/Search Search-related issues that do not fall into other categories v7.0.0 v6.3.0 labels Apr 2, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

@jimczi jimczi added the review label Apr 3, 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.

@jimczi I took a look and found some helper that might be reusable. It does a few more things, so I'm not sure if it good to reuse it though. Would like to hear your opinion.

@@ -264,6 +265,16 @@ private Query applyBoost(Query q, Float boost) {
// Filters unsupported fields if a pattern is requested
// Filters metadata fields if all fields are requested
return resolveMappingField(context, field, 1.0f, !allFields, !multiFields, quoted ? quoteFieldSuffix : null);
} else if (quoted && quoteFieldSuffix != null) {
Copy link
Member

Choose a reason for hiding this comment

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

When looking at how the field suffix is handled in SimpleQueryStringQuery#newPhraseQuery I saw there is a helper called QueryParserHelper.resolveMappingFields which might be reused here. At least the new unit tests pass when using that helper. It does a little more than the code here, so I'm not sure if that would be a good idea though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, I pushed 0abf44c

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, thats what I tried locally. I wasn't sure it works in this case though because it also seems to resolve some regex patterns around wildcards etc... and I thought maybe this isn't wanted here, but glad it can be reused.

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

@jimczi jimczi merged commit 644e5ea into elastic:master Apr 4, 2018
@jimczi jimczi deleted the query_string_quote_field_suffix branch April 4, 2018 15:29
jimczi added a commit that referenced this pull request Apr 4, 2018
This change fixes the handling of the `quote_field_suffix` option on `query_string`
 query. The expansion was not applied to default fields query.

Closes #29324
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Apr 4, 2018
* 6.x:
  Improve similarity integration. (elastic#29187)
  Fix some query extraction bugs. (elastic#29283)
  Fixed quote_field_suffix in query_string (elastic#29332)
  TEST: Update negative byte size setting error msg
  Fix bwc in GeoDistanceQuery serialization (elastic#29325)
martijnvg added a commit that referenced this pull request Apr 5, 2018
* es/master: (68 commits)
  Allow using distance measure in the geo context precision (#29273)
  Disable failing query in QueryBuilderBWCIT.
  Fixed quote_field_suffix in query_string (#29332)
  Use fixture to test repository-url module (#29355)
  Remove undocumented action.master.force_local setting (#29351)
  Enhance error for out of bounds byte size settings (#29338)
  Fix QueryAnalyzerTests.
  Fix HasChildQueryBuilderTests to not use the `classic` similarity.
  [Docs] Correct javadoc of GetIndexRequest (#29364)
  Make TransportRankEvalAction members final
  Add awaits fix for a query analyzer test
  Check presence of multi-types before validating new mapping (#29316)
  Add awaits fix for HasChildQueryBuilderTests
  Remove silent batch mode from install plugin (#29359)
  Align cat thread pool info to thread pool config (#29195)
  Track Lucene operations in engine explicitly (#29357)
  Build: Fix Java9 MR build (#29312)
  Reindex: Fix error in delete-by-query rest spec (#29318)
  Improve similarity integration. (#29187)
  Fix some query extraction bugs. (#29283)
  ...
martijnvg added a commit that referenced this pull request Apr 5, 2018
* es/6.x: (68 commits)
  Add note to migration docs on silent batch mode (#29365)
  Allow using distance measure in the geo context precision (#29273)
  Disable failing query in QueryBuilderBWCIT.
  Improve similarity integration. (#29187)
  Fix some query extraction bugs. (#29283)
  Fixed quote_field_suffix in query_string (#29332)
  TEST: Update negative byte size setting error msg
  Fix bwc in GeoDistanceQuery serialization (#29325)
  Move testMappingConflictRootCause to different class
  Enhance error for out of bounds byte size settings (#29338)
  [Docs] Correct javadoc of GetIndexRequest (#29364)
  Check presence of multi-types before validating new mapping (#29316)
  Make TransportRankEvalAction members final
  Pass through script params in scripted metric agg (#29154)
  Remove silent batch mode from install plugin (#29359)
  Track Lucene operations in engine explicitly (#29357)
  Build: Fix Java9 MR build (#29312)
  Reindex: Fix error in delete-by-query rest spec (#29318)
  Move Nullable into core (#29341)
  [Docs] Correct experimental note formatting
  ...
kcm pushed a commit that referenced this pull request May 9, 2018
This change fixes the handling of the `quote_field_suffix` option on `query_string`
 query. The expansion was not applied to default fields query.

Closes #29324

(cherry picked from commit 644e5ea)
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.3.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Query String Query with quote_field_suffix not using the quote_field_suffix field to search
5 participants