Skip to content

[CPP] Preserve embedded NUL bytes in DICTIONARY strings - #1022

Merged
ColinLeeo merged 1 commit into
apache:developfrom
ColinLeeo:codex/fix-dictionary-embedded-nul
Oct 18, 2026
Merged

ColinLeeo merged 1 commit into
apache:developfrom
ColinLeeo:codex/fix-dictionary-embedded-nul

Conversation

@ColinLeeo

@ColinLeeo ColinLeeo commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

DICTIONARY decoding in C++ truncated TEXT/STRING values at the first NUL byte because read_var_str constructed a C string without its encoded length. Preserve the full byte length, including empty values and embedded/trailing NULs.

Adds a Java-generated binary dictionary golden test, a C++ dictionary round trip, and a var-string regression spanning ByteStream pages.

Validation: 33 byte-stream/dictionary tests passed; C++ Spotless and git diff --check passed. This addresses the dictionary failure exposed by #1021 independently of the compatibility-matrix expansion.

Combined verification with both independent fixes and the #1021 matrix passed: Java-to-C++ 954/954 files and C++-to-Java 918/918 files, including all three scales and six compressors. The SPRINTZ fixtures were regenerated with the corrected writers; the original non-SPRINTZ corpus was retained.

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@ColinLeeo
ColinLeeo requested a lite review from Copilot October 17, 2026 15:53

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@ColinLeeo
ColinLeeo merged commit 963b552 into apache:develop Oct 18, 2026
40 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.

2 participants