Skip to content

Conversation

@liketic
Copy link

@liketic liketic commented Dec 15, 2017

Per @KarstenRauch 's code, I made some minor changes.

Closes #27822

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

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 @liketic ! The changes looks good.
The IT test is not needed, it should be added in the existing rest tests for collapsing (see my comment below).

@@ -0,0 +1,121 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

For a test like this we prefer the rest tests:
https://github.com/elastic/elasticsearch/blob/master/rest-api-spec/src/main/resources/rest-api-spec/test/search/110_field_collapsing.yml
It will start nodes that you can query like a client would do in the real world.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your quick response. I pushed c9b8ee4

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.

Nice, thanks for the quick patch @liketic !

@jimczi jimczi merged commit f5e0932 into elastic:master Dec 15, 2017
jimczi pushed a commit that referenced this pull request Dec 15, 2017
Add version support for inner hits in field collapsing
jimczi pushed a commit that referenced this pull request Dec 15, 2017
Add version support for inner hits in field collapsing
@liketic liketic deleted the bugfix/issues/27822 branch December 16, 2017 07:58
jimczi added a commit that referenced this pull request Dec 18, 2017
jimczi added a commit that referenced this pull request Dec 18, 2017
martijnvg added a commit that referenced this pull request Dec 18, 2017
* es/6.x: (170 commits)
  Allow TrimFilter to be used in custom normalizers (#27758)
  recovery from snapshot should fill gaps (#27850)
  Remove unused class PreBuiltTokenFilters (#27839)
  Reject scroll query if size is 0 (#22552) (#27842)
  Mutes ‘Rollover no condition matched’ YAML test
  Make randomNonNegativeLong() draw from a uniform distribution (#27856)
  Adapt rest test after backport. Relates #27833
  Handle case where the hole vertex is south of the containing polygon(s) (#27685)
  Move range field mapper back to core
  Fix publication of elasticsearch-cli to Maven
  Do not use system properties when building the HttpAsyncClient (#27829)
  Optimize version map for append-only indexing (#27752)
  Add NioGroup for use in different transports (#27737)
  adapt field collapsing skip test version. relates #27833
  Add version support for inner hits in field collapsing (#27822) (#27833)
  Clarify that number of threads is set by packages
  Register HTTP read timeout setting
  Fixes Checkstyle
  Remove `operationThreaded` from Java API (#27836)
  Fixes failing BytesSizeValues tests
  ...
@clintongormley clintongormley added :Search/Search Search-related issues that do not fall into other categories and removed :Inner Hits labels Feb 14, 2018
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.1 v6.2.0 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants