fix: Remove power(decimal, int) code path#22651
Conversation
|
Thanks for the PR, @neilconway . I had similar reflections on Since Nevertheless, other scale-preserving functions like |
|
@theirix Yep, makes sense to me! |
Jefffrey
left a comment
There was a problem hiding this comment.
makes sense; it was a nice idea to have native decimal support but the implementation complexity & edge cases made it a bit of a headache in reality 😅
|
thanks @neilconway & @theirix |
Which issue does this PR close?
power(decimal, integer)overflow for moderate exponents #22480power(decimal, -integer)is incorrect #22510Rationale for this change
power(decimal, int)attempted to computepowerwithout loss of precision. For negative exponents, the code did the computation inFloat64and then cast the result back todecimal. Unfortunately, the previous implementation got this wrong, becausedecimalmight not have enough precision to accurately represent the result. It seems simpler to returnFloat64for the negative exponent case.We could potentially try to return
decimalonly for non-negative exponents andFloat64for negative exponents, but that is complicated, and also means that the code would produce different results for literal arguments vs. columnar arguments, which I think should be avoided.On reflection, it seems simplest to just remove the
power(decimal, int)code path entirely, and havepoweralways returnFloat64. This also fixes another issue in the decimal code path (#22480)What changes are included in this PR?
power(decimal, ...)support; both args will be coerced to Float64 if necessary, and the function will always return Float64power(decimal, integer)overflow for moderate exponents #22480 andpower(decimal, -integer)is incorrect #22510Are these changes tested?
Yes.
Are there any user-facing changes?
Yes,
power(decimal, ...)will now returnFloat64.