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

Handling NULL values in aggregate functions #5

Closed
rausnitz opened this issue Feb 23, 2019 · 3 comments
Closed

Handling NULL values in aggregate functions #5

rausnitz opened this issue Feb 23, 2019 · 3 comments
Labels
enhancement New feature or request

Comments

@rausnitz
Copy link
Sponsor

Fluent 3 currently supports the SQL aggregate functions COUNT(), AVG() , MAX(), MIN(), and SUM(). In each case, Fluent expects a non-optional value. However all of these functions except COUNT() can return a NULL value, in which case Fluent will throw an error.

It would be nice if Fluent mirrored the SQL functions and returned an optional. Alternatively, Fluent could make it easier to avoid this error by allowing you to default to a value such as 0 if the result is NULL.

Example:

Suppose there is a payments table consisting of two fields, userId and amount.

userId amount
1 10.00
1 11.00
1 20.25
2 4.00

In Fluent currently, Payment.query(on: conn).filter(\.userId == 3).sum(\.amount) would result in an error, because the SQL query SELECT SUM(amount) FROM payments WHERE "userId" = 3; would return NULL while Fluent expects a non-optional value.

I think a better way to represent this SQL result would be for Fluent's aggregate functions to return an optional. This would apply to all the aggregate functions except for COUNT(), which always results in a non-optional integer.

Another solution would be for Fluent's aggregate functions to accept an optional default value and use the COALESCE function in SQL to fall back on that value when the aggregate result is NULL. In this case, Payment.query(on: conn).filter(\.userId == 3).average(\.amount, default: 0) would translate to SELECT COALESCE(AVG(amount), 0) FROM payments WHERE "userId" = 3;. I was able to make this solution and submit a PR vapor/fluent#582 but it would have been a breaking change for Fluent 3 and I think having most of the aggregate functions return optionals makes more sense anyway.

@tanner0101 tanner0101 added the enhancement New feature or request label Feb 26, 2019
@tanner0101
Copy link
Member

tanner0101 commented Feb 26, 2019

I've implemented QueryBuilder.max this way as a test, and it works great. I think it makes sense for all NULL-able methods to return an optional. Since it's already fairly easy in Swift to provide a default value for an optional (??, flatMap, etc), I think this should be sufficient. It would be possible though to create additional methods with a required default: param that return non-optional values.

@rausnitz
Copy link
Sponsor Author

👍 Agreed. Looks good!

@tanner0101
Copy link
Member

tanner0101 commented Mar 6, 2020

Fluent 4' query builder supports optionals for all aggregates besides count.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants