#4514 [PATCH] Check and immediately return value from an average if it's already a BigDecimal - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#4514 ✓committed

[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

Neeraj Singh May 1st, 2010 @ 05:20 AM
+1

Looks good to me.
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...
Brian Lopez May 4th, 2010 @ 07:35 PM
new patch per code-review
- new-average-calculation.patch 1.4 KB
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...
Brian Lopez May 4th, 2010 @ 08:23 PM
Attaching the backport for 2.3
- 23-average-calculation.patch 1.3 KB
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.
- type_cast_calculated_value-master.patch 1 KB
- type_cast_calculated_value-2-3-stable.patch 1017 Bytes
Santiago Pastorino May 12th, 2010 @ 05:50 AM
A better master patch here. I will redo the one for 2-3-stable later
- average_things-master.patch 2 KB
Santiago Pastorino May 12th, 2010 @ 05:58 AM
For master without checking types
- average_things-master.patch 1.9 KB
Santiago Pastorino May 12th, 2010 @ 06:02 AM
For 2-3-stable here
- average_things-2-3-stable.patch 1.8 KB
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”
[bulk edit]
You flagged this item as spam.
Andreas Mayer December 2nd, 2010 @ 05:10 PM
May this be related to ticket #6103 ?