This project is archived and is in readonly mode.
[PATCH] Check and immediately return value from an average if it's already a BigDecimal
Reported by Brian Lopez | April 30th, 2010 @ 11:05 PM | in 3.0.2
The return value from an average calculation is assumed to respond to #to_d but the value may already be a BigDecimal to start with. This patch adds a check for that case and returns the value as-is.
Comments and changes to this ticket
-
Andrew White May 3rd, 2010 @ 03:18 PM
I don't think the value is ever a BigDecimal, as it doesn't have a to_d method so would raise a NoMethodError if it ever was.
-
Jeremy Kemper May 3rd, 2010 @ 05:25 PM
- Milestone set to 2.3.6
- State changed from new to open
It is when the driver returns typecasted results, like mysql2.
Brian, this whole method should probably return self for non-Strings.
-
Brian Lopez May 3rd, 2010 @ 07:11 PM
Was hoping to fix and repost the patch this weekend but didn't end up having time. I'll try and get it up today or tomorrow
-
Brian Lopez May 3rd, 2010 @ 10:55 PM
Here's the updated patch, still looking into test failures to see if they're from this or something else
-
Brian Lopez May 4th, 2010 @ 09:30 AM
Ok, here's the patch with updated and passing tests (based on master)
-
Jeremy Kemper May 4th, 2010 @ 07:01 PM
Hey Brian, this changes behavior on nil results too, though. Think it should be confined to just letting pre-typecasted values through.
-
Brian Lopez May 4th, 2010 @ 07:10 PM
Was worried that wouldn't be desired - in our short chat on IRC about it you mentioned returning 0 for nil which is why I put it in there.
It does make sense to me that a sum/avg/count should return a Numeric in all cases, but then again it does seem a better fit to return exactly what the db is giving us (even null).Updated patch coming soon...
-
Repository May 4th, 2010 @ 07:58 PM
- State changed from open to committed
(from [7aad851c2e0d4579aa33a54a069a767b53cca406]) Allow pre-casted values (other than nil) to pass through from calculations un-touched
[#4514 state:committed]
Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/7aad851c2e0d4579aa33a54a069a76... -
Repository May 4th, 2010 @ 08:29 PM
(from [6dbc75fd7625c0248d269cc3127910cb4c888ed9]) Allow pre-casted values (other than nil) to pass through from calculations un-touched
[#4514 state:committed]
Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/6dbc75fd7625c0248d269cc3127910... -
Santiago Pastorino May 10th, 2010 @ 07:45 AM
- Milestone cleared.
- State changed from committed to open
This break test_should_average_field test.
Patchs for master and 2-3-stable added. -
Santiago Pastorino May 12th, 2010 @ 05:50 AM
A better master patch here. I will redo the one for 2-3-stable later
-
Repository May 12th, 2010 @ 06:30 AM
- State changed from open to committed
(from [ad73a3aec4665d8a44060640e51075fd732c33a1]) type_cast_calculated_value refactor: value is never a Fixnum here. Fix test since SQLite returns Float.
[#4514 state:committed]
Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/ad73a3aec4665d8a44060640e51075... -
Repository May 12th, 2010 @ 06:30 AM
(from [5f3bd55726703e08ba595555e1cf428c57832603]) type_cast_calculated_value refactor: value is never a Fixnum here. Fix test since SQLite returns Float.
[#4514 state:committed]
Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
http://github.com/rails/rails/commit/5f3bd55726703e08ba595555e1cf42... -
Jeremy Kemper October 15th, 2010 @ 11:01 PM
- Milestone set to 3.0.2
- Importance changed from to Low
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Attachments
Referenced by
- 4514 [PATCH] Check and immediately return value from an average if it's already a BigDecimal [#4514 state:committed]
- 4514 [PATCH] Check and immediately return value from an average if it's already a BigDecimal [#4514 state:committed]
- 4514 [PATCH] Check and immediately return value from an average if it's already a BigDecimal [#4514 state:committed]
- 4514 [PATCH] Check and immediately return value from an average if it's already a BigDecimal [#4514 state:committed]
- 6103 average() returns integer values instead of decimals Maybe related to #4514?