Skip to content

Conversation

@jimczi
Copy link
Contributor

@jimczi jimczi commented Oct 26, 2017

This change adds some missing options to the expand query that builds the inner hits for field collapsing.
The following options are now applied to the inner_hits query:

  • post_filters
  • preferences
  • routing

Closes #27079
Closes #26649

This change adds some missing options to the expand query that builds the inner hits for field collapsing.
The following options are now applied to the inner_hits query:
 * post_filters
 * preferences
 * routing

Closes elastic#27079
Closes elastic#26649
@jimczi jimczi added :Search/Search Search-related issues that do not fall into other categories >bug v6.1.0 v7.0.0 labels Oct 26, 2017
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, I left some very minor suggestions

if (orig.routing() != null) {
groupRequest.routing(orig.routing());
}
if (orig.searchType() != null) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: looks like searchType cannot be null

.indicesOptions(orig.indicesOptions())
.requestCache(orig.requestCache());
groupRequest.setMaxConcurrentShardRequests(orig.getMaxConcurrentShardRequests());
if (orig.preference() != null) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: maybe the null check isn't necessary

if (orig.preference() != null) {
groupRequest.preference(orig.preference());
}
if (orig.routing() != null) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: maybe the null check isn't necessary

@jimczi jimczi merged commit d1acf44 into elastic:master Oct 26, 2017
@jimczi jimczi deleted the field_collapsing_expand_options branch October 26, 2017 15:02
jimczi added a commit that referenced this pull request Oct 26, 2017
* Apply missing request options to the expand phase

This change adds some missing options to the expand query that builds the inner hits for field collapsing.
The following options are now applied to the inner_hits query:
 * post_filters
 * preferences
 * routing

Closes #27079
Closes #26649
jasontedor added a commit to olcbean/elasticsearch that referenced this pull request Oct 30, 2017
* master: (63 commits)
  [Docs] Fix note in bucket_selector
  [Docs] Fix indentation of examples (elastic#27168)
  [Docs] Clarify `span_not` query behavior for non-overlapping matches (elastic#27150)
  [Docs] Remove first person "I" from getting started (elastic#27155)
  [Docs] Correct link target for datatype murmur3 (elastic#27143)
  Fix division by zero in phrase suggester that causes assertion to fail
  Enable Docstats with totalSizeInBytes for 6.1.0
  Adds average document size to DocsStats (elastic#27117)
  Upgrade Painless from ANTLR 4.5.1-1 to  ANTLR 4.5.3. (elastic#27153)
  Exists template needs a template name (elastic#25988)
  [Tests] Fix occasional test failure due to two random values being the same
  Fix beidermorse phonetic token filter for unspecified `languageset` (elastic#27112)
  Fix max score tracking with field collapsing (elastic#27122)
  [Doc] Add Ingest CSV Processor Plugin to plugin as a community plugin (elastic#27105)
  Removed the beta tag from cross-cluster search
  fixed typo in ConstructingObjectParse (elastic#27129)
  Allow for the Painless Definition to have multiple instances (elastic#27096)
  Apply missing request options to the expand phase (elastic#27118)
  Only pull SegmentReader once in getSegmentInfo (elastic#27121)
  Fix BWC for discovery stats
  ...
@jmuscireum
Copy link

Hi! Would you mind backporting this fix to 5.6? #26649 is a major bug for us and we are unfortunately not ready for 6.x yet.

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.1.0 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants