Fix type casting to Decimal from Float with large precision #16333
Conversation
|
@joker1007 won't this patch lead to problems when reading data from the database, which actually make use of the precision specified on the column? |
|
@senny
But, precision larger than |
|
@joker1007 right thank you for clarifying. /cc @sgrif |
| @@ -14,7 +14,9 @@ def type_cast_for_schema(value) | |||
| private | |||
|
|
|||
| def cast_value(value) | |||
| if value.is_a?(::Numeric) || value.is_a?(::String) | |||
| if value.is_a?(::Float) | |||
| BigDecimal(value, [precision.to_i, ::Float::DIG + 1].min) | |||
There was a problem hiding this comment.
Can we do this in the constructor instead?
def initialize(*)
super
if precision > ::Float::DIG + 1
@precision = ::Float::DIG + 1
endThere was a problem hiding this comment.
Never mind I just realized how stupid that is. Let's at least pull this out to a method. My preference would still be for an if statement, but I don't feel super strongly about it.
There was a problem hiding this comment.
Only When the value is Float, it is necessary to limit precision.
If value is other type, precision value has reason.
limitation by constructor seems not good.
|
I was hoping to remove |
| @@ -14,7 +14,9 @@ def type_cast_for_schema(value) | |||
| private | |||
|
|
|||
| def cast_value(value) | |||
| if value.is_a?(::Numeric) || value.is_a?(::String) | |||
| if value.is_a?(::Float) | |||
There was a problem hiding this comment.
Once we have 3 cases we usually use a case statement instead.
There was a problem hiding this comment.
I followed the style of base code.
I prefer to case statement too.
Thanks!
|
Do we really want to do this? It's raising an exception for a reason. Edit: Yes, yes we do. |
| BigDecimal(value, precision.to_i) | ||
| elsif value.respond_to?(:to_d) | ||
| when ->(v) { v.respond_to?(:to_d) } |
There was a problem hiding this comment.
This proc feels wrong to me. I think I'd prefer this to be an if statement inside of the else case.
|
@sgrif |
|
@joker1007 looks good, can you squash your commits and add an entry to the CHANGELOG? |
When I defines large precision column at RDBMS, I assigns float value, raise ArgumentError (precision too large).
|
@senny OK, I'm done. |
…with_large_precision Fix type casting to Decimal from Float with large precision Conflicts: activerecord/CHANGELOG.md
|
@joker1007 thank you 💛 |
When I define large precision column at RDBMS,
I assign float value, raises ArgumentError (precision too large).
Cause of this probrem:
precisionis derived from database column definition.precisionis larger thanFloat::DIG + 1,BigDecimal.newraises ArgumentErrorI think it is inconvenient.
this commit fixes it.