[PR] fix(parquet): avoid false overflow in DeltaBitPackDecoder::skip for INT32 [arrow-rs]

["prostomarkeloff (via GitHub)" <gi...@apache.org>]

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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org