SignificantTerms aggregation #847
Conversation
|
Changes Unknown when pulling 91065a6 on miniplay:significant-terms-aggregation into * on ruflin:master*. |
| * @see https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-bucket-significantterms-aggregation.html#_custom_background_context | ||
| */ | ||
| public function setBackgroundFilter(AbstractQuery $query) { | ||
| $this->setParam("background_filter", $query->toArray()); |
There was a problem hiding this comment.
Missed return statement. All setters must implement fluent interface.
|
@rmruano Could you look into my eyes and say that every existing parameter of Terms aggregation can be applied to SignificantTerms? If it's not, class should be extended from AbstractSimpleAggregation. |
| * @param AbstractQuery $query | ||
| * @see https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-bucket-significantterms-aggregation.html#_custom_background_context | ||
| */ | ||
| public function setBackgroundFilter(AbstractQuery $query) { |
There was a problem hiding this comment.
According to parameter name, shouldn't here be AbstractFilter indeed?
rmruano
force-pushed
the
significant-terms-aggregation
branch
from
91065a6 to
c48da91
Compare
|
@im-denisenko Thanks for your feedback. Apparently, the only missing feature is I've also fixed SignificantTerms::setBackgroundFilter() with your suggestions. |
|
Changes Unknown when pulling c48da91 on miniplay:significant-terms-aggregation into * on ruflin:master*. |
|
Merged, thanks! |
SignificantTerms aggregation is almost identical to Terms Aggregation. Extended it and added a convenience method for setting a background filter. Simple tests added, although they may not be very good examples: SignificantTerms doesn't shine on automatically generated datasets.