Lstarsky0 commented on issue #10787: URL: https://github.com/apache/arrow-rs/issues/10787#issuecomment-5383985542
Both panics are the bookkeeping counters overflowing, not the decimal value. `result` goes through `mul_wrapping`/`add_wrapping` the whole way and never panics on its own. There are four unchecked sites in this function, and the repro covers two of them: - `digits: u8` (`parse.rs:917`, incremented at `:953` and `:988`) — panics on the 256th significant digit. That's `"1".repeat(256)`; 255 is fine. - `exp: i16` (`:808`, `exp *= 10` at `:854`) — that's `1e99999`. - `exp as u8 + digits` at `:1023`, inside the non-e-notation precision check. This one fires *earlier* than the first: `parse_decimal::<Decimal128Type>(&"1".repeat(246), 38, 10)` panics at 246 digits, and 245 doesn't, because `10 + 246` leaves `u8`. - `(digits as i16 + exp)` at `:889`, the e-notation precision check. `1e32767` survives the exponent parse (it's exactly `i16::MAX`) and panics here instead. The release side is worse than one bad value. `digits` wraps mod 256, so the guard at `:1030` (`digits > precision`) accepts anything whose significant-digit count is `<= precision` mod 256. With precision 38: ``` 254, 255 ones Err 256..294 ones Ok(-75618303760208547436305468318170713657) 295..511 ones Err 512..550 ones Ok(same value) 551 ones Err ``` so it's a periodic acceptance window rather than a tail case. Worth keeping in mind that `:1030` can't be the backstop for this by itself, since the number it tests is the one that wrapped. -- 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]
