Repository navigation
fix(writer): narrow and cascade decimal_byte_parts' value child, as Rust does - #647
Merged
Merged
Conversation
…ust does Rust's DecimalScheme narrows a decimal to the smallest signed integer type holding its values (narrowed_decimal) and compresses the most significant part with compress_child, so frame of reference and bit-packing take it. DecimalBytePartsEncodingEncoder had no encodeCascade and always wrote the part as a plain i64 vortex.primitive. The encoder now narrows the part to i8/i16/i32/i64 (signed only, as Rust), records that type in the metadata, and returns the part as an open cascade child. TPC-H from tpchgen-cli, vortex-java before -> after (Rust): sf10 lineitem 3,378 -> 1,796 MB (1,825) sf10 orders 519 -> 448 MB (512) sf10 partsupp 311 -> 264 MB (263) sf10 customer 105 -> 97 MB (95) sf10 part 64 -> 52 MB (56) Import time unchanged (lineitem sf10: 19.4 -> 19.6 s). DecimalBytePartsInteropIntegrationTest: for five TPC-H-shaped decimal columns the data column's encoding tree equals vortex-jni's (it fails without this change), and vortex-jni reads the Java file in full, filtered and row-range scans. Closes #644 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011fRbYryTxfcu5j2CXwkSzd
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #644.
Rust's
DecimalSchemenarrows a decimal to the smallest signed integer type holding its values (narrowed_decimal) and compresses the most significant part withcompress_child, so frame of reference and bit-packing take it.DecimalBytePartsEncodingEncoderhad noencodeCascadeand always wrote the part as a plain i64vortex.primitive.Change
try_downcast), record that type in the metadata, and return it as an open cascade childvortex.decimal), as Rust leaves it un-narrowedBefore → after (TPC-H from
tpchgen-cli, vortex-java Parquet import; Rust = vortex Python bindings):Same at sf1. Import time unchanged (lineitem sf10: 19.4 → 19.6 s).
Tests
DecimalBytePartsInteropIntegrationTest: for five TPC-H-shaped decimal columns the data column's encoding tree equals vortex-jni's (fails without this change), and vortex-jni reads the Java file in full, filtered and row-range scans.Docs:
docs/compatibility.mdrow and theVortexWritercomment updated.Reviewed independently (vortex-reviewer agent): reader side and zone stats verified; its should-fix items (the <2-value parity case, stale docs/comment, test FQNs and a weak assertion) are in this commit. Its nullable-decimal finding is #646 (a nullable decimal goes through
vortex.masked, which is also why the stats tables still differ). Found while measuring: #645 (Snappy Parquet unreadable).🤖 Generated with Claude Code
https://claude.ai/code/session_011fRbYryTxfcu5j2CXwkSzd