comphead opened a new pull request, #12094: URL: https://github.com/apache/arrow-rs/pull/12094
# Which issue does this PR close? - Relates to apache/datafusion#26687. This picks up #11552 by @Dandandan with the same fix, rebased on `main`. The benchmarks are split out into #12093. # Rationale for this change `StreamDecoder::with_skip_validation(true)` only applies to dictionary batches, so record batches are still fully validated. `StreamReader`, `FileDecoder` and the `arrow-flight` `FlightDataDecoder` all pass the flag through for record batches too. DataFusion's spill reader has called `StreamDecoder::new().with_skip_validation(true)` since apache/datafusion#21882. Because of this bug, every spilled batch is validated again when it is read back, including a UTF-8 check of every string. That undoes apache/datafusion#15454. # What changes are included in this PR? - Pass `skip_validation` to `RecordBatchDecoder` in `StreamDecoder::decode`. - Cover `StreamDecoder` in `expect_ipc_validation_error`. # Are these changes tested? Yes. `expect_ipc_validation_error` now checks that `StreamDecoder` rejects invalid data by default and accepts it when validation is skipped. Without the fix, the five `test_validation_of_invalid_*` tests fail on the new check. Benchmarks from #12093 on an M5 Max, `main` against this PR: | Benchmark | main | this PR | Change | |---|---|---|---| | `StreamDecoder/no_validation/read_10` | 129.0 µs | 6.18 µs | −95.2% | | `StreamDecoder/read_10` | 128.9 µs | 128.8 µs | no change | The `StreamReader` and `FileReader` benchmarks do not change. With arrow patched to this branch, DataFusion's `spill_io/StreamReader/read_100` drops from 9.59 ms to 8.48 ms (−11.5%). That benchmark reads 100 spilled batches of 8192 rows through `StreamDecoder`. After #12093 merges, rebasing this PR lets `run benchmark ipc_reader` confirm these numbers on the shared runner. # Are there any significant user-facing changes? No API changes, and validation is still on by default. Callers that pass `true` now skip validation for record batches too, as documented. AI disclosure: The fix, the test and this description were written with Claude Code. I have reviewed every change and the benchmark results. -- 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]
