Skip to content

GH-54885: [C++][Compute] Use 64-bit offsets for basic row encoding - #55170

Open
zanmato1984 wants to merge 1 commit into
apache:mainfrom
zanmato1984:fix/basic-row-encoding-64bit-upstream
Open

zanmato1984 wants to merge 1 commit into
apache:mainfrom
zanmato1984:fix/basic-row-encoding-64bit-upstream

Conversation

@zanmato1984

@zanmato1984 zanmato1984 commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Rationale for this change

Basic hash joins store encoded build-side keys and payloads in contiguous RowEncoder buffers. Signed 32-bit offsets and byte counts overflow when the accumulated encoded data exceeds 2 GiB, even when the individual input arrays are valid. This can cause a vector length exception and terminate the calling Python process.

For large_string keys, each encoded value includes a one-byte null marker and an eight-byte length prefix, explaining why the failure can occur before the raw string data reaches 2 GiB.

What changes are included in this PR?

  • Use 64-bit byte offsets and lengths throughout RowEncoder, including cumulative append sizes and the row length returned by encoded_row().
  • Widen shared KeyEncoder length counters and remove narrowing conversions for large scalar and array values.
  • Update basic Grouper batch offsets, persistent unique-key offsets, and encoded key lengths consistently.
  • Add opt-in large-memory regressions for encoder length calculation and appends, Grouper buffers, and hash-join keys and payloads.

Row IDs, group IDs, and per-column encoded length formats are unchanged.

Are these changes tested?

Added six opt-in large-memory regression tests covering encoder length calculation and cumulative appends, basic Grouper batch and unique-key buffers, and hash-join keys and payloads.

Are there any user-facing changes?

Basic hash joins using large_string or large_binary columns can accumulate more than 2 GiB of encoded build-side keys or payloads without overflowing their cumulative byte offsets.

The memory cost is in the offset arrays: each offset grows from 4 to 8 bytes. A RowEncoder storing N rows has N + 1 offsets, so their element storage increases by 4 × (N + 1) bytes—approximately 3.8 MiB per million rows, per encoder, excluding spare vector capacity and allocator overhead. Separate key and payload encoders each incur this cost. This doubles offset-array storage, not total join memory.

Basic Grouper likewise uses four additional bytes per persistent unique-key offset and per temporary batch offset. The encoded key/value bytes and per-column length prefixes are unchanged. There are no public API changes, and existing row-count and group-count limits remain unchanged.

This PR contains a "Critical Fix". It addresses a crash on valid input when encoded build-side data exceeds the signed 32-bit offset range.

Was AI used for this PR?

The author conducted a thorough review with AI assistance.

PR code and description written by:

  • Human
  • AI

Reviewed before submission by:

  • Human
  • AI
  • Not reviewed

Signed-off-by: Rossi Sun <zanmato1984@gmail.com>
Copilot AI balanced review requested due to automatic review settings October 24, 2026 06:29
@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #54885 has been automatically assigned in GitHub to PR creator.

Copilot AI 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.

🟢 Approval recommended

Static review found no blocking issues in the consistent widening or regression coverage; the opt-in large-memory tests were not executed.

No open issues 🎉

What changed in this PR

Fixes the 2 GiB encoded-buffer overflow behind GH-54885 by widening internal byte offsets and lengths to 64 bits in basic row encoding and grouping. Row IDs, group IDs, and per-column encoding formats remain unchanged.

Changes:

  • Widens RowEncoder offsets and shared KeyEncoder length counters.
  • Updates basic Grouper temporary and persistent buffer offsets consistently.
  • Adds six opt-in large-memory regressions covering encoders, grouping, and join keys and payloads.
File Description
cpp/​src/​arrow/​compute/​row/​row_encoder_internal.h Widens encoder interfaces, row lengths, and stored offsets.
cpp/​src/​arrow/​compute/​row/​row_encoder_internal.cc Uses 64-bit length calculations and cumulative append sizes.
cpp/​src/​arrow/​compute/​row/​row_encoder_internal_test.cc Tests large value lengths and cumulative appends beyond 2 GiB.
cpp/​src/​arrow/​compute/​row/​grouper.cc Widens batch and persistent unique-key offsets.
cpp/​src/​arrow/​compute/​row/​grouper_test.cc Tests batch and unique-key buffers beyond 2 GiB.
cpp/​src/​arrow/​acero/​hash_join_node_test.cc Tests oversized build-side key and payload buffers.

Review effort: Balanced

@zanmato1984 zanmato1984 added the Critical Fix Bugfixes for security vulnerabilities, crashes, or invalid data. label Oct 24, 2026
@zanmato1984
zanmato1984 requested a review from pitrou October 24, 2026 06:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review Awaiting review Component: C++ Critical Fix Bugfixes for security vulnerabilities, crashes, or invalid data.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants