Skip to content

fix(arrow-ipc): honor skip_validation for record batches in StreamDecoder - #12094

Merged
Rich-T-kid merged 2 commits into
apache:mainfrom
comphead:stream-decoder-skip-validation
Nov 5, 2026
Merged

fix(arrow-ipc): honor skip_validation for record batches in StreamDecoder#12094
Rich-T-kid merged 2 commits into
apache:mainfrom
comphead:stream-decoder-skip-validation

Conversation

@comphead

@comphead comphead commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

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.

@comphead
fix(arrow-ipc): honor skip_validation for record batches in StreamDec…
1789eac 
…oder

`StreamDecoder::with_skip_validation` only reached dictionary batches, so
record batches were still validated. Pass the flag to `RecordBatchDecoder`
as `StreamReader` and `FileDecoder` do, and cover `StreamDecoder` in
`expect_ipc_validation_error`.
@github-actions github-actions Bot added the arrow-ipc arrow-ipc crate label Nov 5, 2026
This was referenced Nov 5, 2026
Closed
Open
Rich-T-kid
Rich-T-kid approved these changes Nov 5, 2026
View reviewed changes

@Rich-T-kid Rich-T-kid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @comphead, ill wait to run the benchmarks before merging

@Rich-T-kid

Rich-T-kid commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

run benchmark ipc_reader
env:
BENCH_FILTER: StreamDecoder

@adriangbot

adriangbot commented Nov 5, 2026

Copy link
Copy Markdown

🤖 Benchmark starting (GKE) | trigger

Target: ipc_reader

Sharding factor: 1 (1 workers).

Comparing stream-decoder-skip-validation (2812df6) to 71a9574 (merge-base) diff

Run configuration 
run benchmark ipc_reader
env:
  BENCH_FILTER: StreamDecoder
shards: 1

Results will be posted when all workers finish.

File an issue against this benchmark runner

@Rich-T-kid

Rich-T-kid commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

run benchmark ipc_reader
env:
BENCH_FILTER: StreamDecoder

@adriangbot

adriangbot commented Nov 5, 2026

Copy link
Copy Markdown

🤖 Benchmark starting (GKE) | trigger

Target: ipc_reader

Sharding factor: 1 (1 workers).

Comparing stream-decoder-skip-validation (2812df6) to 71a9574 (merge-base) diff

Run configuration 
run benchmark ipc_reader
env:
  BENCH_FILTER: StreamDecoder
shards: 1

Results will be posted when all workers finish.

File an issue against this benchmark runner

@adriangbot

adriangbot commented Nov 5, 2026

Copy link
Copy Markdown

🤖 Benchmark completed (GKE) | trigger

Target: ipc_reader

Comparing stream-decoder-skip-validation (2812df6) to 71a9574 (merge-base) diff

Run configuration 
run benchmark ipc_reader
env:
  BENCH_FILTER: StreamDecoder
shards: 1
Details 
group                                                   base                                   changed
-----                                                   ----                                   -------
arrow_ipc_reader/StreamDecoder/no_validation/read_10    18.75  310.0±28.22µs        ? ?/sec    1.00     16.5±0.26µs        ? ?/sec
arrow_ipc_reader/StreamDecoder/read_10                  1.00   309.9±28.55µs        ? ?/sec    1.01   312.8±28.55µs        ? ?/sec
Per-runner information
ipc_reader — shard 1/1

Node: gk3-benchmark-cluster-nap-hn34dqn7-a7b7f634-dk47

Instance: c4a-highmem-16 (12 vCPU / 65 GiB)

uname:

Linux bench-c6670922980-4530-7sgsk 6.12.105+ #1 SMP Sat Sep 19 07:55:25 UTC 2026 aarch64 GNU/Linux

BENCH_COMMAND:

cargo bench --features=arrow,async,test_common,experimental,object_store --bench ipc_reader
CPU Details (lscpu) 
Architecture:                            aarch64
CPU op-mode(s):                          64-bit
Byte Order:                              Little Endian
CPU(s):                                  16
On-line CPU(s) list:                     0-15
Vendor ID:                               ARM
Model name:                              Neoverse-V2
Model:                                   1
Thread(s) per core:                      1
Core(s) per cluster:                     16
Socket(s):                               -
Cluster(s):                              1
Stepping:                                r0p1
BogoMIPS:                                2000.00
Flags:                                   fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache:                               1 MiB (16 instances)
L1i cache:                               1 MiB (16 instances)
L2 cache:                                32 MiB (16 instances)
L3 cache:                                80 MiB (1 instance)
NUMA node(s):                            1
NUMA node0 CPU(s):                       0-15
Vulnerability Gather data sampling:      Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit:             Not affected
Vulnerability L1tf:                      Not affected
Vulnerability Mds:                       Not affected
Vulnerability Meltdown:                  Not affected
Vulnerability Mmio stale data:           Not affected
Vulnerability Reg file data sampling:    Not affected
Vulnerability Retbleed:                  Not affected
Vulnerability Spec rstack overflow:      Not affected
Vulnerability Spec store bypass:         Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1:                Mitigation; __user pointer sanitization
Vulnerability Spectre v2:                Mitigation; CSV2, BHB
Vulnerability Srbds:                     Not affected
Vulnerability Tsa:                       Not affected
Vulnerability Tsx async abort:           Not affected
Vulnerability Vmscape:                   Not affected
Resource Usage

base (merge-base)

Metric Value
Wall time 30.0s
Peak memory 13.4 MiB
Avg memory 10.9 MiB
CPU user 22.1s
CPU sys 0.0s
Peak spill 0 B

branch

Metric Value
Wall time 25.0s
Peak memory 17.4 MiB
Avg memory 11.8 MiB
CPU user 19.0s
CPU sys 0.0s
Peak spill 0 B

File an issue against this benchmark runner

@adriangbot

adriangbot commented Nov 5, 2026

Copy link
Copy Markdown

🤖 Benchmark completed (GKE) | trigger

Target: ipc_reader

Comparing stream-decoder-skip-validation (2812df6) to 71a9574 (merge-base) diff

Run configuration 
run benchmark ipc_reader
env:
  BENCH_FILTER: StreamDecoder
shards: 1
Details 
group                                                   base                                   changed
-----                                                   ----                                   -------
arrow_ipc_reader/StreamDecoder/no_validation/read_10    18.77  310.2±25.97µs        ? ?/sec    1.00     16.5±0.25µs        ? ?/sec
arrow_ipc_reader/StreamDecoder/read_10                  1.00   309.7±28.59µs        ? ?/sec    1.00   309.8±28.41µs        ? ?/sec
Per-runner information
ipc_reader — shard 1/1

Node: gk3-benchmark-cluster-nap-hn34dqn7-a7b7f634-gdwg

Instance: c4a-highmem-16 (12 vCPU / 65 GiB)

uname:

Linux bench-c6670924317-4531-2slts 6.12.105+ #1 SMP Sat Sep 19 07:55:25 UTC 2026 aarch64 GNU/Linux

BENCH_COMMAND:

cargo bench --features=arrow,async,test_common,experimental,object_store --bench ipc_reader
CPU Details (lscpu) 
Architecture:                            aarch64
CPU op-mode(s):                          64-bit
Byte Order:                              Little Endian
CPU(s):                                  16
On-line CPU(s) list:                     0-15
Vendor ID:                               ARM
Model name:                              Neoverse-V2
Model:                                   1
Thread(s) per core:                      1
Core(s) per cluster:                     16
Socket(s):                               -
Cluster(s):                              1
Stepping:                                r0p1
BogoMIPS:                                2000.00
Flags:                                   fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 sm3 sm4 asimddp sha512 sve asimdfhm dit uscat ilrcpc flagm sb paca pacg dcpodp sve2 sveaes svepmull svebitperm svesha3 svesm4 flagm2 frint svei8mm svebf16 i8mm bf16 dgh rng bti
L1d cache:                               1 MiB (16 instances)
L1i cache:                               1 MiB (16 instances)
L2 cache:                                32 MiB (16 instances)
L3 cache:                                80 MiB (1 instance)
NUMA node(s):                            1
NUMA node0 CPU(s):                       0-15
Vulnerability Gather data sampling:      Not affected
Vulnerability Indirect target selection: Not affected
Vulnerability Itlb multihit:             Not affected
Vulnerability L1tf:                      Not affected
Vulnerability Mds:                       Not affected
Vulnerability Meltdown:                  Not affected
Vulnerability Mmio stale data:           Not affected
Vulnerability Reg file data sampling:    Not affected
Vulnerability Retbleed:                  Not affected
Vulnerability Spec rstack overflow:      Not affected
Vulnerability Spec store bypass:         Mitigation; Speculative Store Bypass disabled via prctl
Vulnerability Spectre v1:                Mitigation; __user pointer sanitization
Vulnerability Spectre v2:                Mitigation; CSV2, BHB
Vulnerability Srbds:                     Not affected
Vulnerability Tsa:                       Not affected
Vulnerability Tsx async abort:           Not affected
Vulnerability Vmscape:                   Not affected
Resource Usage

base (merge-base)

Metric Value
Wall time 25.0s
Peak memory 13.5 MiB
Avg memory 11.0 MiB
CPU user 20.8s
CPU sys 0.0s
Peak spill 0 B

branch

Metric Value
Wall time 25.0s
Peak memory 17.4 MiB
Avg memory 11.8 MiB
CPU user 19.0s
CPU sys 0.0s
Peak spill 0 B

File an issue against this benchmark runner

Rich-T-kid
Rich-T-kid approved these changes Nov 5, 2026
View reviewed changes

@Rich-T-kid Rich-T-kid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice 😄, thanks @comphead

arrow_ipc_reader/StreamDecoder/no_validation/read_10    18.77  310.2±25.97µs        ? ?/sec    1.00     16.5±0.25µs        ? ?/sec

@Rich-T-kid
Rich-T-kid added this pull request to the merge queue Nov 5, 2026
@Jefffrey Jefffrey added the bug label Nov 5, 2026
@comphead

comphead commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @Rich-T-kid I'll create a 60.2.0 backport

Merged via the queue into apache:main with commit 56b1895 Nov 5, 2026
23 checks passed
@mbutrovich

mbutrovich commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Thanks @Rich-T-kid I'll create a 60.2.0 backport

I don’t think there’s a backport to do. I think 60.2 will be released from the main branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Rich-T-kid Rich-T-kid Rich-T-kid approved these changes

Assignees

No one assigned

Labels

arrow-ipc arrow-ipc crate bug performance

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@comphead @Rich-T-kid @adriangbot @mbutrovich @Jefffrey