Repository navigation
Keep the DecimalByteParts MSP at its logical integer width - #11354
Draft
connortsui20 wants to merge 9 commits into
Draft
connortsui20 wants to merge 9 commits into
connortsui20 wants to merge 9 commits into
Conversation
Device-resident Widen arrays cannot use the CPU fallback. Restore their logical width on the device so narrow integer and decimal children can execute through CUDA. Progress towards #11284. Signed-off-by: Connor Tsui <connor.tsui20@gmail.com> Signed-off-by: Alexander Droste <alexander.droste@protonmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
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.
Summary
#10905 decoded a narrow MSP into a decimal
Widen, which needed a precision label and amin_maxscan to choose it. CodSpeed reported decoding 4.7x to 8.4x slower with that design. This PR removes the scan by fixing the MSP's logical dtype from the decimal precision: the signed integer at the canonical widthW(p), capped ati64. Narrower storage is an integerWiden. SinceW(p)is the smallest width that holds10^p - 1, every value of a narrower signed integer fitsDecimal(p), so decoding only needs to sign-extend.This replaces the closed #10905 and #10992, covering CD5 with CD7 folded in. It depends on #11303 and must merge after it. The base remains
ct/cuda-widenuntil then.Changes
The writer stores the
Widenchild rather than the wrapper, so the wire remains readable by old readers.On read, a narrower stored MSP gets an integer
Widenwithout a scan. Only a wider legacy MSP gets an eager checked cast on the host, rather than trusting that its valid values fit. Device buffers are rejected for that legacy conversion.CPU and GPU decoding peel the
Widenand widen straight to the canonical decimal width, avoiding a copy to the logical MSP width followed by another widening copy.The compressor uses
WidenArray::encodeto narrow every MSP, including one next to lower parts, then compresses the narrow child before rewrapping it. Passing the wrapper tocompress_childwould canonicalize it and lose the narrow storage.split_decimalalways returns the MSP at its logical width rather than inheriting the decimal buffer's width, so decimals with narrower or wider storage still encode.With lower parts,
compare_constant_wideloses its whole-answer shortcut when the constant's MSP word is outside the narrow child's range. The follow-up is to check against theWidenchild's range. Lower parts remain unsupported on the GPU, and the CUDA tests only run in GPU CI. CodSpeed'sdbp_computedecodebenchmark should show a single widening copy with no scan. Benchmark results for this PR are pending.New tests cover legacy decimal storage in both width directions, decoding a wrapped MSP with and without lower parts, wire round trips, checked overflow, and invalid values in null lanes. The golden trees now expose the integer
Widennode.API Changes
DecimalByteParts::try_newandtry_new_with_lower_parts, andDecimalBytePartsData::validate, now require the logical MSP dtype fixed by the precision, with matching decimal nullability. Lower parts must be non-nullableu64, and their count is bounded by the precision.split_decimalnow takes an explicit lower-part count. Callers can obtain the smallest count from the newlower_part_countfunction, which considers valid values only.vortex_array::arrays::decimal::narrowed_decimalis removed._benchmarking::assemble_decimalhelper now requires the validated DBP part layout and always returns canonical-width decimal storage.Validation
Checked tip
8025ec3490on #11303 at712a4a337e.cargo clippywith--all-targets --all-features -- -D warningspassed forvortex-decimal-byte-parts,vortex-btrblocks,vortex-compat,vortex-array,vortex-arrow,vortex-duckdb,vortex, andvortex-cuda.cargo +nightly-2026-09-10 fmt --all --checkpassed.cargo nextest run -p vortex-decimal-byte-parts -p vortex-btrblocks -p vortex-compatpassed all 717 tests, with none skipped.uv run vortex-test/compat-gen/scripts/compat.py check --mode lastpassed all 43 v0.93.0 files, with no failures or skips. Both old DBP files passed, including the narrow and wide legacy MSP cases indecimal_byte_parts_narrow_msp.vortex.