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

[5.6] Math DB functions should return numeric type #22640

Closed
wants to merge 1 commit into from
Closed

[5.6] Math DB functions should return numeric type #22640

wants to merge 1 commit into from

Conversation

antonkomarev
Copy link
Contributor

@antonkomarev antonkomarev commented Jan 4, 2018

I've found one interesting thing when started to declare static types in one of my packages. Maybe I'm missing something, but it's not right that database query math functions like avg and sum returns string instead of float or int.

I understand that min & max functions are math too, but they could be used to get DATE & TIMESTAMP types. Is there any use cases for avg or sum functions where string could be returned?

Average dates are getting by converting them to Unix timestamps, calculating average value and then converting them to date value back.

Notice that we don't need this check: return $result ?: 0;. It's done in numericAggregate method.

@antonkomarev
Copy link
Contributor Author

antonkomarev commented Jan 4, 2018

One more function is count, but I was unsure if we need to change it. It's using very frequently and it could be bad for performance.

@GrahamCampbell
Copy link
Member

I think we did this before, then had to revert?

@taylorotwell
Copy link
Member

Yes pretty certain this already had to be reverted once. If you can find out and fix why let me know in another PR.

@antonkomarev
Copy link
Contributor Author

antonkomarev commented Jan 5, 2018

There were issues with min and max functions. But test for this case already in place and they are failing if these functions will return numeric values: #14991

@taylorotwell I didn't opened new PR as you said, because I don't see anything to fix here. I've changed only avg and sum functions.

Initially changed here: #14793
Reverted here: #14994

@antonkomarev antonkomarev deleted the fix/math-db-query-functions-return-type branch September 10, 2019 19:22
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

Successfully merging this pull request may close these issues.

3 participants