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

Document behaviour of aggregators when extracted value is null #9858

Closed
jerrinot opened this issue Feb 10, 2017 · 2 comments
Closed

Document behaviour of aggregators when extracted value is null #9858

jerrinot opened this issue Feb 10, 2017 · 2 comments

Comments

@jerrinot
Copy link
Contributor

@jerrinot jerrinot commented Feb 10, 2017

@jerrinot jerrinot added this to the 3.8 milestone Feb 10, 2017
@jerrinot jerrinot changed the title Check behaviour of aggregators when extracted value is null Document behaviour of aggregators when extracted value is null Feb 13, 2017
@jerrinot jerrinot modified the milestones: 4.0, 3.8 Feb 13, 2017
@jerrinot
Copy link
Contributor Author

@jerrinot jerrinot commented Feb 13, 2017

After some research and a discussion with @ahmetmircik and @vbekiaris I came to this conclusion:

  • Builtin aggregations such as Average or Sum assume an aggregated value is non-null.

  • If an aggregated attribute is a null then e.g. Average aggregation will throw an Exception.
    This can be prevented by using a predicate to only aggregate entries where the attribute is not null.

We could implement auto-skipping of null values in aggregations, but:

  1. It make certain aggregations impossible ("Count how many entries have attribute foo set to null?")
  2. It's conflating entry selection and entry processing: We can already do map.aggregate(count("age"), new SqlPredicate("name = 'Joe' and age is not null") if needed.

So the remaining bit is to document this behaviour in contract of the built-in aggregations.

@jerrinot jerrinot modified the milestones: 3.9, 4.0, 3.8.1 Feb 13, 2017
@tombujok tombujok self-assigned this Feb 14, 2017
@tombujok
Copy link
Contributor

@tombujok tombujok commented Feb 20, 2017

Closed via

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

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.