-
-
Notifications
You must be signed in to change notification settings - Fork 210
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
use decimal type for arithmetic functions #1244
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall; I just had a few questions I wanted to make sure I understood.
I suspect that AVG
needs the same treatment as SUM
. We can handle that in a separate PR, but would be good to check that out while we're in here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good. This turned out to be trickier than I was expecting, so thanks for all the extra attention to detail. 😁
fixes dolthub/dolt#4269
sum will be either float64 or decimal.Decimal type, but return type is decided on the type of the child expression of sum expression. This causes some test results to be modified into int from float.
the test results in
decimal.Decimal
type is hard to get the exact matching using its convert method, so result is checked in string format by usingStringFixed()
method of the decimal.Decimal result value.