Skip to content

Fix Utf8View/BinaryView variadicBufferCounts Int64 deserialization - #615

Merged
quinnj merged 2 commits into
apache:mainfrom
bdklahn:fix-variadic-buffer-counts
Nov 11, 2026
Merged

quinnj merged 2 commits into
apache:mainfrom
bdklahn:fix-variadic-buffer-counts

Conversation

@bdklahn

@bdklahn bdklahn commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

RecordBatch.variadicBufferCounts is defined as [long] (Int64) in the Apache Arrow Message.fbs spec, but was deserialized as FlatBuffers.Array{Int32}. When reading tables with multiple Utf8View/BinaryView columns (e.g. from Polars), reading 32-bit elements split each 64-bit buffer count into two 32-bit words, shifting buffer offsets for subsequent columns and causing out-of-bounds reads or segmentation faults in Arrow.View getindex:

signal 11 (2): Segmentation fault: 11
getindex at ./essentials.jl:975 [inlined]
getindex at Arrow/src/arraytypes/views.jl:61 [inlined]

Also handle 4-byte-aligned Int64 vectors in FlatBuffers.Array{T} when the vector payload following the 4-byte UOffsetT length prefix is not 8-byte aligned.

RecordBatch.variadicBufferCounts is defined as [long] (Int64) in the
Apache Arrow Message.fbs spec, but was deserialized as FlatBuffers.Array{Int32}.
When reading tables with multiple Utf8View/BinaryView columns (e.g. from Polars),
reading 32-bit elements split each 64-bit buffer count into two 32-bit words,
shifting buffer offsets for subsequent columns and causing out-of-bounds reads
or segmentation faults in Arrow.View getindex:

  signal 11 (2): Segmentation fault: 11
  getindex at ./essentials.jl:975 [inlined]
  getindex at Arrow/src/arraytypes/views.jl:61 [inlined]

Also handle 4-byte-aligned Int64 vectors in FlatBuffers.Array{T} when the
vector payload following the 4-byte UOffsetT length prefix is not 8-byte aligned.
Copilot AI balanced review requested due to automatic review settings November 6, 2026 03:21

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

A misaligned variadicBufferCounts payload is copied in full for every view column, causing quadratic metadata copying and substantial memory overhead for wide batches.

1 open issue
What changed in this PR

This PR corrects Arrow record-batch metadata decoding for Utf8View/BinaryView columns, addressing incorrect buffer offsets that can cause out-of-bounds reads or crashes.

Changes:

  • Deserialize variadicBufferCounts as Int64, matching the Arrow specification.
  • Copy misaligned FlatBuffers vector data into aligned storage.
  • Add a regression test for the counts’ element type and decoded values.
File Description
test/​runtests.jl Tests Int64 variadic buffer count deserialization.
src/​metadata/​Message.jl Corrects the counts’ element type from Int32 to Int64.
src/​FlatBuffers/​table.jl Adds an aligned-copy fallback for misaligned vectors.

Review effort: Balanced

Comment thread src/FlatBuffers/table.jl Outdated
data = if UInt(ptr) % Base.datatype_alignment(S) == 0
unsafe_wrap(Base.Array, ptr, len)
else
copy(reinterpret(S, @view bytes(t)[(a + 1):(a + len * sizeof(S))]))
@codecov-commenter

codecov-commenter commented Nov 10, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.14%. Comparing base (3712291) to head (94027fa).
⚠️ Report is 57 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #615      +/-   ##
==========================================
+ Coverage   87.43%   88.14%   +0.71%     
==========================================
  Files          26       28       +2     
  Lines        3288     3485     +197     
==========================================
+ Hits         2875     3072     +197     
  Misses        413      413              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@quinnj quinnj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[review generated via AI]

Reviewed at 94027fa. The fix is correct and I would merge it as is. Thank you for tracking this down.

Reproduced on main (71df78e), fixed here. Julia 1.12.6 with --check-bounds=yes (and the IPC file again on 1.10), files from pyarrow 25.0.1 and Polars 1.43.2 with three view columns (string_view, binary_view, string_view) that each carry long values:

  • main: BoundsError: attempt to access 0-element Vector{Vector{UInt8}} at index [1] from views.jl:47 on the first long value of the second view column. This happens for the IPC file, the IPC stream, and the Polars stream.
  • This branch: all 2000 rows of every column match what pyarrow wrote, through both Arrow.Table and Arrow.Stream.

Smallest repro I found (a 552-byte stream):

import pyarrow as pa, pyarrow.ipc as ipc
t = pa.table({"a": pa.array(["a-long-string-over-12-bytes", "x"], pa.string_view()),
              "b": pa.array(["b-long-string-over-12-bytes", "y"], pa.string_view())})
with ipc.new_stream("two_views.arrows", t.schema) as w:
    w.write_table(t)
Arrow.Table("two_views.arrows").b[1]   # main: BoundsError; this branch: "b-long-string-over-12-bytes"

The counts [1, 1] are read as Int32 [1, 0], so column b gets zero data buffers.

What I checked

  • Message.fbs declares variadicBufferCounts: [long]. build(::ViewTypes) is the only reader. Arrow.jl 2.x does not write view types, so there is no writer side to fix.
  • The other FlatBuffers vectors already use the right width (typeIds is [int]). Nothing in src/ dispatches on the old three-parameter FlatBuffers.Array.
  • The alignment fallback is needed. unsafe_wrap throws ArgumentError: unsafe_wrap: pointer ... is not properly aligned to 8 bytes when an Int64 vector does not sit on an 8-byte boundary. I read the counts with the stream placed at every byte shift from 0 to 15. All sixteen are correct on this head (a Vector at shifts 0 and 8, a ReinterpretArray elsewhere).
  • Full suite locally on Julia 1.12.6 and 1.10: the same results as main plus the two new passing tests. CI is green on this head.

Suggestions, none blocking

  1. The new test always lands on the aligned branch (the vector sits at pos = 32 in the built bytes), so the reinterpret fallback has no coverage. This loop covers both branches and passes on this head:

    bytes = Vector(FB.finishedbytes(b))
    for shift = 0:8
        rb = FB.getrootas(Arrow.Meta.RecordBatch, vcat(zeros(UInt8, shift), bytes), shift)
        @test rb.variadicBufferCounts == Int64[0, 7, 0]
    end
  2. An end-to-end test would guard the behavior users see. The 552-byte stream above could be checked in next to reject_reason_trimmed.arrow with @test Arrow.Table(path).b[1] == "b-long-string-over-12-bytes".

  3. The extra type parameter costs a little on very wide tables. Reading 2000 Int64 columns in 5 batches allocates 33.0 MiB here against 30.1 MiB on main (deterministic). Wall time was within noise on my machine. I don't think it matters for this fix.

  4. This PR and #616 both edit the #511 testset, so they conflict with each other in test/runtests.jl. Whichever lands second needs a rebase.

@quinnj
quinnj merged commit 46932f1 into apache:main Nov 11, 2026
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants