bharadwaj-pendyala commented on PR #10911: URL: https://github.com/apache/arrow-rs/pull/10911#issuecomment-5464534508
> took a glance at the issue + PR. the unit test look fine but im a little confused from the description of the PR > > ``` > Op::Div scales the left operand before dividing: result_scale = s1 + 4, then mul_pow = result_scale - s1 + s2. For large operand scales, mul_pow includes all of s2, so l * 10^mul_pow overflows even when the quotient fits. > ``` > > could you give an example of this to showcase exactly where the overflow would occure as well as what this new algorithm would avoid it. Take 3.0 / 6.0 at scale 37 on Decimal128. The old code multiplies the left side by 10^38 before dividing. That number has 76 digits, way past what i128 can hold, so it errors out. But the actual answer, 0.5, only needs 38 digits. Plenty of room. The overflow happens in a number we throw away, not in the answer itself. `scaled_div` skips that step. It does long division the way you'd do it by hand: divide, get a remainder, bring down a zero, repeat. At no point does it build that huge intermediate number, so it never overflows on cases where the real answer would have fit. Test for this is `test_decimal128_div_wide_intermediate` in the diff. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
