Skip to content

Commit

Permalink
Apply scale before precision when coercing floats to decimal
Browse files Browse the repository at this point in the history
Since precision is always larger than scale, it can actually change
rounding behavior. Given a precision of 5 and a scale of 3, when you
apply the precision of 5 to `1.25047`, the result is `1.2505`, which
when the scale is applied would be `1.251` instead of the expected
`1.250`.

This issue appears to only occur with floats, as scale doesn't apply to
other numeric types, and the bigdecimal constructor actually ignores
precision entirely when working with strings. There's no way we could
handle this for the "unknown object which responds to `to_d`" case, as
we can't assume an interface for applying the scale.

Fixes #24235
  • Loading branch information
sgrif committed Mar 24, 2016
1 parent a12ad8a commit c7d3bd4
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 2 deletions.
12 changes: 10 additions & 2 deletions activemodel/lib/active_model/type/decimal.rb
Expand Up @@ -29,12 +29,12 @@ def cast_value(value)
end
end

scale ? casted_value.round(scale) : casted_value
apply_scale(casted_value)
end

def convert_float_to_big_decimal(value)
if precision
BigDecimal(value, float_precision)
BigDecimal(apply_scale(value), float_precision)
else
value.to_d
end
Expand All @@ -47,6 +47,14 @@ def float_precision
precision.to_i
end
end

def apply_scale(value)
if scale
value.round(scale)
else
value
end
end
end
end
end
7 changes: 7 additions & 0 deletions activemodel/test/cases/type/decimal_test.rb
Expand Up @@ -52,6 +52,13 @@ def test_changed?
assert_not type.changed?(5.0, 5.0, '5.0')
assert_not type.changed?(-5.0, -5.0, '-5.0')
end

def test_scale_is_applied_before_precision_to_prevent_rounding_errors
type = Decimal.new(precision: 5, scale: 3)

assert_equal BigDecimal("1.250"), type.cast(1.250473853637869)
assert_equal BigDecimal("1.250"), type.cast("1.250473853637869")
end
end
end
end

0 comments on commit c7d3bd4

Please sign in to comment.