Repository navigation
Export decimal Widen arrays to Arrow with one copy - #10994
Draft
connortsui20 wants to merge 2 commits into
Draft
connortsui20 wants to merge 2 commits into
connortsui20 wants to merge 2 commits into
Conversation
This was referenced Oct 29, 2026
Merging this PR will not alter performance2 warnings
Comparing |
connortsui20
force-pushed
the
ct/decimal-widen
branch
from
October 29, 2026 20:20
29d4a4a to
9c42fc9
Compare
connortsui20
force-pushed
the
ct/widen-arrow-export
branch
from
October 29, 2026 20:23
49a4f52 to
cab830d
Compare
A decimal Widen at precision 18 or less exports as Decimal128, and today the exporter copies its child to the canonical width before it copies again to the Arrow width. Add a Widen case under precision 18, where that double copy happens, and one under precision 38, where the canonical width already equals the Arrow width, so the next change can be measured against both. Signed-off-by: Connor Tsui <[email protected]>
Arrow exports every precision up to 38 as Decimal128, and the decimal exporter executed its input to the canonical DecimalArray first. A decimal Widen at precision 18 or less therefore copied its child twice: once into the canonical width, and again into the Arrow width. Stop at a Widen and export its child with the precision and scale of the logical dtype, so each value is copied once. The converters take that dtype apart from the values, so no DecimalArray ever pairs it with the narrower child width. Struct, list, dictionary and run-end children reach this exporter too, so they get the single copy as well. The converters narrowed every row, so a null row whose value does not fit in the Arrow width failed the export. Executing the Widen hid this, because the canonical copy skips null rows. Narrow only the valid rows and write zero into the null rows, like the canonical copy does. This also fixes the same failure for a plain DecimalArray stored wider than its canonical width. Signed-off-by: Connor Tsui <[email protected]>
connortsui20
force-pushed
the
ct/decimal-widen
branch
from
October 29, 2026 21:32
9c42fc9 to
fdf9ac2
Compare
connortsui20
force-pushed
the
ct/widen-arrow-export
branch
from
October 29, 2026 21:33
cab830d to
dacf961
Compare
This branch has not been deployed
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.
Tracking Issue: #10701
Stacked on #10946. It needs only decimal children in
Widen, so it can merge before the rest of the decimalWidenstack, and it has no release gate.Summary
Arrow exports every decimal up to precision 38 as
Decimal128, but the exporter first executed its input to the canonicalDecimalArray. A decimalWidenat precision 18 or less therefore copied its values twice, once into the canonicali64width and again intoi128. After #10905, every DecimalByteParts column with a narrow MSP decodes to aWiden, so this is the common export path for decimals that DataFusion reads from a file.With this change, exporting
widen_i32_p18costs the same as exporting the canonical array (27.12 µs, down from 42.06 µs).Changes
The exporter stops at a
Widenand converts its child straight into the Arrow width, with the precision and scale of the logical dtype. The converters take the logical dtype separately from the values, so the export never builds aDecimalArraywhose dtype is wider than its values, which CD8a will forbid. Struct, list, dictionary and run-end children reach the same exporter. Above precision 18 the canonical width is alreadyi128, so those exports don't change.When a child is stored wider than the Arrow width, the export narrows it. A null row whose value didn't fit used to fail the whole export. Narrowing now skips null rows and writes zero into them, as
copy_to_canonical_widthdoes, which also fixes the same failure for a plainDecimalArray.The tests check that a
Widenexport equals the canonical export at the largest precision of each child width, for every Arrow decimal type the parent precision fits, with garbage in the null rows.Benchmark results (Apple M4, medians)
cargo bench -p vortex-arrow --bench to_arrow -- decimal_export, 100,000 rows exported toDecimal128. Before is the benchmark commit of this PR and after is its tip, so the comparison isolates this layer. Other jobs shared the machine during the runs.widen_i32_p18widen_i64_p38canonical_i64_p18widen_i32_p18is the case this PR changes.widen_i64_p38andcanonical_i64_p18copy once both before and after, so they are controls.API Changes
No public API changes. Exporting a decimal stored wider than the Arrow width no longer fails on a null row whose value does not fit.