Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
zoul0813 opened this issue May 14, 2018 · 1 comment

Comments

@zoul0813
Copy link
Contributor

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

@zoul0813
Copy link
Contributor Author

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant