Repository navigation
Read old vortex.decimal nodes into the canonical array or a Widen - #10995
Draft
connortsui20 wants to merge 3 commits into
Draft
connortsui20 wants to merge 3 commits into
connortsui20 wants to merge 3 commits into
Conversation
This was referenced Oct 29, 2026
Merging this PR will regress 2 benchmarks1 warning
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing |
connortsui20
force-pushed
the
ct/decimal-widen
branch
from
October 29, 2026 20:20
29d4a4a to
9c42fc9
Compare
connortsui20
force-pushed
the
ct/decimal-read-plugin
branch
from
October 29, 2026 20:24
bad5407 to
d2a4285
Compare
A vortex.decimal node from an older writer can store its values at a width other than the canonical width of its dtype. The read plugin of the next commit converts such a node, and it must never panic on a malformed one. Move the decoding of a node into StoredDecimal, which checks the metadata, the buffer count, the dtype, the alignment and the validity in one place. It also checks that the buffer holds exactly len values of the stored width. Before, only the array validation after decoding caught a buffer of another length, and a read that converts the node needs the check before it reads any value. Signed-off-by: Connor Tsui <[email protected]>
Older writers stored a decimal dtype at any width that held its values, so old files hold vortex.decimal nodes at a non-canonical width. After this commit, no read of a vortex.decimal node returns a decimal leaf at a non-canonical width. DecimalPlugin takes over the vortex.decimal ID. A canonical node reads as before, with no copy and no scan, and a device buffer stays on the device. Any other node goes through WidenArray::from_stored_decimal, which returns the canonical array or a decimal Widen. Nodes wider than the canonical width follow the same rule, so they can also become a Widen. from_stored_decimal executes the validity on the host, so the plugin first copies it there with the new ArrayRef::try_into_host_sync. CUDA has no Widen kernel, so old nodes read on the host even on CUDA paths. The Decimal vtable now rejects a non-canonical width with an error that names DecimalPlugin, and the default session registers the plugin in its place. Signed-off-by: Connor Tsui <[email protected]>
The legacy decimal fixtures write vortex.decimal columns at widths other than the canonical width. Their round-trip test asserted that a read keeps the stored width, which DecimalPlugin no longer does. The test now asserts the form of each read column. A column stored at the canonical width reads unchanged, with its raw slots. Any other column reads as a Widen over the smallest precision that holds its valid values, or as the canonical array when that precision has the canonical width. A slow reference function gives that precision. The values are compared with the canonical expected arrays. 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/decimal-read-plugin
branch
from
October 29, 2026 21:33
d2a4285 to
3cfb191
Compare
connortsui20
changed the base branch from
ct/decimal-widen
to
ct/decimal-widen-stored
October 29, 2026 21:33
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 #11005, the layer of the decimal
Widenstack that addsfrom_stored_decimal. It does not need #11006.Summary
Older writers stored a decimal dtype at any width that held its values, so old files contain
vortex.decimalnodes at a width other than the canonical width of their dtype. Once everyDecimalArraymust have the canonical width, these nodes can't read back as stored, so they have to convert on read. They convert throughWidenArray::from_stored_decimal, the same path that DecimalByteParts decoding uses (#10905), so every kind of old decimal data reads as the canonical array or a decimalWiden.Do not merge until 0.91.0 publishes the CD2 compat fixtures. Then run
uv run vortex-test/compat-gen/scripts/compat.py check --mode laston this branch. The aim is to ship this in the same release as #10905 and #10992.Changes
DecimalPlugintakes over thevortex.decimalID in the default session. A node at the canonical width reads as before, with no copy and no scan, and its buffer stays on the device. Any other node reads into the canonical array or aWidenover the smallest precision that holds its valid values. The plugin can't use stored statistics (the deserializer attaches them after the plugin returns), so an old node costs one scan of its values.Old nodes are rare, since the compressor has put every decimal that fits in 64 bits into DecimalByteParts since 2025-05. So instead of adding a device path (CUDA has no
Widenkernel), the plugin copies the buffers and validity of an old node to the host. The new crate-privateArrayRef::try_into_host_synccopies the validity tree, and a failed copy returns an error. Open question for a CUDA reviewer: does a GPU scan over such a column still work?Decimal::deserializenow rejects a non-canonical width. Both paths decode through a sharedStoredDecimal, which checks the buffer length, alignment and validity against the dtype, so a malformed node returns an error instead of panicking.Follow-up: in Python, an old narrow node now reads as a decimal
Widen, which has no Python subclass, so it isn't aDecimalArraythere.API Changes
Reading an old
vortex.decimalnode returns the canonical array or a decimalWideninstead of aDecimalArrayat the stored width. Addsvortex_array::arrays::decimal::DecimalPlugin, which the defaultArraySessionregisters in place ofDecimal. A session that registers onlyDecimalrejects old nodes.