Rails Ruby on Rails → 2.1.2

Comments and changes to this ticket

• You flagged this item as spam.
  

  crafterm September 18th, 2008 @ 08:40 AM

  • Tag changed from “2.1.1, activerecord, bug” to “2.1.1, activerecord, bug, patch”

  Patch to fix and update tests

  • 0001-Ensure-that-Calculations-sum-avg-return-BigDecimal-i.patch 2.1 KB
• 

  Tarmo Tänav September 19th, 2008 @ 05:05 AM

  • 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 September 19th, 2008 @ 08:04 AM

  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 September 19th, 2008 @ 08:12 AM

  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 September 19th, 2008 @ 08:13 AM

  • Milestone set to “2.1.2”
• 

  Michael Koziarski September 24th, 2008 @ 05:37 PM

  • Assigned user set to “Pratik”

  So which changeset introduced this bug? The more focussed fix sounds correct to me

• 

  Michael Koziarski September 24th, 2008 @ 05:53 PM

  Was it this one?

  http://github.com/rails/rails/co...

  Is it broken in edge rails too?

• 

  Andrew White September 25th, 2008 @ 06:52 AM

  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 September 26th, 2008 @ 05:34 PM

  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 October 4th, 2008 @ 08:12 PM

  • State changed from “new” to “resolved”

  (from [78feaf6be75a5fcb766a7d1dcedd52e890561081]) Ensure Model.sum and Model.avg typecast appropriately. [#1066 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 October 4th, 2008 @ 08:14 PM

  (from [9599948fbcd67c1c2c5fecc2dca148e998479e33]) Ensure Model.sum and Model.avg typecast appropriately. [#1066 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...