GH-51135: [C++][Parquet] Avoid misaligned stores when reading BYTE_ARRAY decimals

[wgtmac]

Rationale for this change

The BYTE_ARRAY decimal converter always clears 16 bytes through uint64_t stores. After Decimal32/64 support was added, Decimal32 output slots can be only 4-byte aligned, causing undefined behavior.

What changes are included in this PR?

Replace the fixed uint64_t stores with std::memset(out_ptr, 0, type_length).

Are these changes tested?

Yes. The existing Decimal32/64/128/256 BYTE_ARRAY tests all pass. No new test is needed because TestReadDecimals.Decimal32ByteArray directly exercises this path.

Are there any user-facing changes?

No, this is a bug fix.

AI usage: OpenAI Codex was used to investigate and draft this change. The patch was reviewed and tested locally.

• GitHub Issue: [C++][Parquet] Avoid misaligned stores when reading BYTE_ARRAY decimals #51135

[github-actions]

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

[Copilot]

🟢 Approval recommended

The change directly resolves the misaligned-store UB risk with a safe, already-included <cstring>-backed approach and is consistent with existing usage in the same file.

Pull request overview

Fixes undefined behavior in the Parquet BYTE_ARRAY-to-decimal conversion path by avoiding potentially misaligned uint64_t stores when initializing output buffers for Decimal32/64/128/256, aligning with the UBSan issue in #51135.

Changes:

• Replace fixed uint64_t zeroing stores with std::memset(out_ptr, 0, type_length) in the BYTE_ARRAY decimal converter.

File summaries

File	Description
cpp/src/parquet/arrow/reader_internal.cc	Uses std::memset to zero decimal output slots safely regardless of alignment, preventing misaligned stores for Decimal32 BYTE_ARRAY reads.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[HuaHuaY]

LGTM.

But I’m wondering: given that unit tests exist, why hasn't Valgrind ever detected this buffer overflow?

[HuaHuaY]

given that unit tests exist, why hasn't Valgrind ever detected this buffer overflow?

It may be that RoundCapacity allocates more memory; thus, while a buffer overflow occurs logically, it does not occur physically.

[HuaHuaY]

Thanks to @wgtmac for fixing this issue.