Skip to content

Conversation

@mayya-sharipova
Copy link
Contributor

Closes #24855

@mayya-sharipova mayya-sharipova requested a review from jimczi July 5, 2018 00:22
@mayya-sharipova mayya-sharipova added the :Search/Search Search-related issues that do not fall into other categories label Jul 5, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

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

It looks good @mayya-sharipova but I wonder if the second collapse should be under inner_hits ? I know we discussed the proposed format already but I forgot that we can have multiple inner_hits in a single collapse so it would be more flexible and consistent to define the collapse field there ?

@mayya-sharipova
Copy link
Contributor Author

@jimczi Thanks Jim, I will try to implement collapse under inner_hits

@mayya-sharipova
Copy link
Contributor Author

mayya-sharipova commented Jul 6, 2018

@jimczi I have modified the PR to make the second level collapse under inner_hits. Now potentially inner_hits for nested and parent/child operations can have collapse field, but I think it is harmless, as they will ONLY be processed if there is a main collapse field. What do you think is it fine this way?

Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

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

Thanks @mayya-sharipova, it looks great !

@mayya-sharipova mayya-sharipova merged commit 80492ca into elastic:master Jul 13, 2018
@mayya-sharipova mayya-sharipova deleted the two-levels-field-collapsing branch July 13, 2018 15:42
mayya-sharipova added a commit that referenced this pull request Jul 13, 2018
Put second level collapse under inner_hits

Closes #24855
dnhatn added a commit that referenced this pull request Jul 13, 2018
* 6.x:
  Watcher: Make settings reloadable (#31746)
  [Rollup] Histo group config should support scaled_floats (#32048)
  lazy snapshot repository initialization (#31606)
  Add secure setting for watcher email password (#31620)
  Watcher: cleanup ensureWatchExists use (#31926)
  Add second level of field collapsing (#31808)
  Added lenient flag for synonym token filter (#31484) (#31970)
  Test: Fix a second case of bad watch creation
  [Rollup] Use composite's missing_bucket (#31402)
  Docs: Restyled cloud link in getting started
  Docs: Change formatting of Cloud options
  Re-instate link in StringFunctionUtils javadocs
  Correct spelling of AnalysisPlugin#requriesAnalysisSettings (#32025)
  Fix problematic chars in javadoc
  [ML] Move open job failure explanation out of root cause (#31925)
  [ML] Switch ML native QA tests to use a 3 node cluster (#32011)
jimczi added a commit that referenced this pull request Jul 16, 2018
Mpdreamz added a commit to elastic/elasticsearch-net that referenced this pull request Oct 1, 2018
russcam pushed a commit to elastic/elasticsearch-net that referenced this pull request Oct 17, 2018
russcam pushed a commit to elastic/elasticsearch-net that referenced this pull request Oct 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>enhancement :Search/Search Search-related issues that do not fall into other categories v6.4.0 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants