prostomarkeloff opened a new pull request, #11864: URL: https://github.com/apache/arrow-rs/pull/11864
Found with [find-dup-defs](https://github.com/prostomarkeloff/find-dup-defs) using its `converge` pass, which reports behavioural divergences between near-duplicate functions. It flagged `DeltaBitPackDecoder`'s `get` and `skip` as near-duplicates that diverge in the `bit_width == 0` arithmetic: `get` advances the value in `T`'s own width, while `skip` does the multiply in `i64` and converts back with `from_i64`. ## Which issue does this PR close? None filed yet — happy to open an issue first if that's preferred. ## Rationale In the `bit_width == 0` branch, `DeltaBitPackDecoder::skip` advances `last_value` by `n * min_delta`. It computes this product in `i64` and then does `T::T::from_i64(total)`, which returns `None` when the product does not fit in `T` (e.g. `i32`) and surfaces as `General("delta*n overflow in skip")`: ```rust let total = min_delta.wrapping_mul(mini_block_to_skip as i64); let step = T::T::from_i64(total) .ok_or_else(|| general_err!("delta*n overflow in skip"))?; ``` `get()` performs the equivalent arithmetic in `T`'s own width with wrapping and never rejects such data, so `skip()` fails on inputs that `get()` decodes fine. This is reachable whenever a reader skips values in an INT32 `DELTA_BINARY_PACKED` column (e.g. a `RowSelection` or page offset), making otherwise-valid files unreadable along that path. ## What changes Do the multiply in `T`'s width with wrapping, matching `get()`: take the count as `T` via `from_usize`, then `self.min_delta.wrapping_mul(&step_count)`. ## Are there any user-facing changes? No API change; files that previously errored on skip now decode. ## Tests Added four tests in `encodings::decoding::tests` that assert `get()` and `skip()` agree on INT32 data triggering the overflow (large per-step deltas spanning the i32 range; alternating `i32::MIN`/`0`). Each fails before this change and passes after; `cargo test -p parquet --lib encodings::decoding` is green. Disclosure: parts of this change were generated with the help of an AI tool; I have reviewed the change and the tests and understand each line. -- 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]
