Skip to content

Keep the DecimalByteParts MSP at its logical integer width - #11354

Draft
connortsui20 wants to merge 9 commits into
developfrom
ct/dbp-integer-child
Draft

connortsui20 wants to merge 9 commits into
developfrom
ct/dbp-integer-child

Conversation

@connortsui20

Copy link
Copy Markdown
Member

Summary

#10905 decoded a narrow MSP into a decimal Widen, which needed a precision label and a min_max scan 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 width W(p), capped at i64. Narrower storage is an integer Widen. Since W(p) is the smallest width that holds 10^p - 1, every value of a narrower signed integer fits Decimal(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-widen until then.

Changes

The writer stores the Widen child rather than the wrapper, so the wire remains readable by old readers.

On read, a narrower stored MSP gets an integer Widen without 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 Widen and widen straight to the canonical decimal width, avoiding a copy to the logical MSP width followed by another widening copy.

The compressor uses WidenArray::encode to narrow every MSP, including one next to lower parts, then compresses the narrow child before rewrapping it. Passing the wrapper to compress_child would canonicalize it and lose the narrow storage.

split_decimal always 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_wide loses its whole-answer shortcut when the constant's MSP word is outside the narrow child's range. The follow-up is to check against the Widen child's range. Lower parts remain unsupported on the GPU, and the CUDA tests only run in GPU CI. CodSpeed's dbp_compute decode benchmark 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 Widen node.

API Changes

  • DecimalByteParts::try_new and try_new_with_lower_parts, and DecimalBytePartsData::validate, now require the logical MSP dtype fixed by the precision, with matching decimal nullability. Lower parts must be non-nullable u64, and their count is bounded by the precision.
  • split_decimal now takes an explicit lower-part count. Callers can obtain the smallest count from the new lower_part_count function, which considers valid values only.
  • vortex_array::arrays::decimal::narrowed_decimal is removed.
  • The public, hidden _benchmarking::assemble_decimal helper now requires the validated DBP part layout and always returns canonical-width decimal storage.
Validation

Checked tip 8025ec3490 on #11303 at 712a4a337e.

  • cargo clippy with --all-targets --all-features -- -D warnings passed for vortex-decimal-byte-parts, vortex-btrblocks, vortex-compat, vortex-array, vortex-arrow, vortex-duckdb, vortex, and vortex-cuda.
  • cargo +nightly-2026-09-10 fmt --all --check passed.
  • cargo nextest run -p vortex-decimal-byte-parts -p vortex-btrblocks -p vortex-compat passed all 717 tests, with none skipped.
  • uv run vortex-test/compat-gen/scripts/compat.py check --mode last passed all 43 v0.93.0 files, with no failures or skips. Both old DBP files passed, including the narrow and wide legacy MSP cases in decimal_byte_parts_narrow_msp.vortex.
  • CUDA runtime tests require GPU CI and were not run locally.

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>
@connortsui20 connortsui20 added the changelog/break A breaking API change label Nov 9, 2026
Base automatically changed from ct/cuda-widen to develop November 10, 2026 10:58

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/break A breaking API change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant