ElasticsearchDSL icon indicating copy to clipboard operation
ElasticsearchDSL copied to clipboard

Bucketing\TermsAggregation does not support filtering, min doc count, etc

Open zoul0813 opened this issue 6 years ago • 1 comments

I'm working on updating this now as I need support for this in my current project. I'll submit a PR shortly.

zoul0813 avatar May 14 '18 14:05 zoul0813

OK - Just noticed that I can call "addParameter" on the TermsAggregation object before adding it to the aggregation set ... however, this seems clunky as I can't just create my aggregation quickly from the constructor.

I'm thinking it might look cleaner if all of the aggregations supported a simple array ... so calling them would look like this;

$agg = new TermsAggregation($name, $params);

We'd eliminate the $field parameter common to most of the classes as it's not available to all

The $params value would then just be looped through, and call addParameter($k, $v) in the AbstractAggregation class ...

foreach($params as $k=>$v) {
  $this->addParameter($k, $v);
}

This would introduce breaking changes though, and I'm not sure if that's something you're interested in dealing with?

I'd be more than happy to work on refactoring the aggregation code to support this style, as well as taking the range aggregations and making either a trait or abstract class for them to share (looks like addRange is repeated a few times, and each one is implemented slightly different but they all do the same thing).

zoul0813 avatar May 14 '18 14:05 zoul0813