This project is archived and is in readonly mode.
Associations#sum no longer returns a BigDecimal
-
crafterm
- Tag changed from 2.1.1, activerecord, bug to 2.1.1, activerecord, bug, patch
Patch to fix and update tests
-
Tarmo Tänav
- Milestone cleared.
Are you sure that in 2.1.0 it returned a BigDecimal? The tests in the attached patch seem to fail when run on 2.1.0 without the change to calculations.rb. Looking through the history of type_cast_calculated_value there don't appear to be any references to BigDecimal or to_d.
-
Andrew White
Looking at the commit history it used to be the case that sum was using the default behaviour to typecast to the column type which is where the confusion is arising.
-
Tarmo Tänav
Aha, indeed that makes sense, but this means it may not be a good idea to make everything BigDecimal, in 2.1.2. Though we may have to restore the typecast behavior.
-
Tarmo Tänav
- Milestone set to 2.1.2
-
Michael Koziarski
- Assigned user set to Pratik
So which changeset introduced this bug? The more focussed fix sounds correct to me
-
Andrew White
Actually it was this one. Looks like it was introduced to make sure sum returned zero when getting empty results from the database.
Your one was introduced to fix the first one typecasting to integer.
The way I've handled this in the past in my own apps is to add a to_d method to NilClass, Fixnum, BigDecimal and Bignum and then just call to_d on the result of the sum method. This way it doesn't matter if there are no items in the sum result set.
The problem with type casting to float for decimal columns and then back to decimal is that you may introduce floating point precision errors. My view would be that if we are doing a straightforward sum on a column then typecast to that column's type otherwise return a string and leave it to the developer to cast to whatever they want. This would also apply to average as well.
As for edge, it has exactly the same behaviour.
-
Michael Koziarski
OK,
so we need a fix for this as it's blocking 2.1.2, or it could always slip to 2.1.3.
Any volunteers to fix up the typecasting and nil handling?
-
Repository
- State changed from new to resolved
(from [78feaf6be75a5fcb766a7d1dcedd52e890561081]) Ensure Model.sum and Model.avg typecast appropriately. [#1066 Associations#sum no longer returns a BigDecimal state:resolved]
Model.sum delegates typecasting to the column being summed. If that's not feasible, returns a string. Model.avg always returns big decimal. http://github.com/rails/rails/co...
-
Repository
(from [9599948fbcd67c1c2c5fecc2dca148e998479e33]) Ensure Model.sum and Model.avg typecast appropriately. [#1066 Associations#sum no longer returns a BigDecimal state:resolved]
Model.sum delegates typecasting to the column being summed. If that's not feasible, returns a string. Model.avg always returns big decimal. http://github.com/rails/rails/co...
