Repository navigation
Bound decimal storage by precision - #10305
joseph-isaacs wants to merge 13 commits into
Conversation
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
0fff52c to
ec2b1b5
Compare
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
Merging this PR will not alter performance3 warnings, 1 note
Comparing Footnotes
|
…-contract Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
develop now initialises ArrayParts with their slots directly, so the precision-bound regression test passes the slots to the constructor instead of calling the removed with_slots helper. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015zQPnFWRi7CWXHpgBE8CiF
Storage is now bounded by precision, so tests that stored values wider than the precision-derived width can no longer be constructed. Move the numeric overflow tests to inputs that overflow within bounded storage, expect i8 storage for a decimal(2, 0) byte-parts split, and drop the row key-width test whose narrowing path is unreachable from a valid array. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015zQPnFWRi7CWXHpgBE8CiF
The checked constructors now fail when storage is wider than the type required by the precision. Importers that receive wider storage, the Arrow decimal conversions and legacy serialized arrays, narrow explicitly through DecimalData::narrow_to_precision before constructing the array. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015zQPnFWRi7CWXHpgBE8CiF
Polar Signals Profiling ResultsLatest Run
Previous Runs (1)
Powered by Polar Signals Cloud |
commented
Oct 9, 2026
Benchmarks: PolarSignals Profiling 📖Commits: PR datafusion / vortex-file-compressed / ns (1.003x ➖, 0↑ 0↓)
No file size changes detected. |
commented
Oct 9, 2026
Benchmarks: String Encoding 📖Commits: PR vortex / vortex-file-compressed / ms (0.999x ➖, 0↑ 0↓)
vortex / vortex-file-compressed / % (1.000x ➖, 0↑ 0↓)
|
commented
Oct 9, 2026
Benchmarks: FineWeb NVMe 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.026x ➖, 0↑ 1↓)
datafusion / vortex-compact / ns (1.006x ➖, 1↑ 1↓)
datafusion / parquet / ns (1.005x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.902x ➖, 3↑ 1↓)
duckdb / vortex-compact / ns (0.961x ➖, 1↑ 0↓)
duckdb / parquet / ns (0.997x ➖, 0↑ 0↓)
No file size changes detected. |
commented
Oct 9, 2026
Benchmarks: TPC-H SF=1 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.997x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (1.003x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.000x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.004x ➖, 0↑ 1↓)
duckdb / vortex-compact / ns (0.993x ➖, 1↑ 1↓)
duckdb / parquet / ns (1.003x ➖, 0↑ 0↓)
No file size changes detected. |
commented
Oct 9, 2026
Benchmarks: Clickbench Sorted on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.007x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (0.987x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.996x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.001x ➖, 2↑ 1↓)
duckdb / vortex-compact / ns (1.002x ➖, 1↑ 1↓)
duckdb / parquet / ns (1.031x ➖, 0↑ 2↓)
File Size Changes (200 files changed, -0.0% overall, 96↑ 104↓)
Totals:
|
commented
Oct 9, 2026
Benchmarks: FineWeb S3 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.903x ➖, 2↑ 0↓)
datafusion / vortex-compact / ns (1.020x ➖, 0↑ 1↓)
datafusion / parquet / ns (1.003x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.981x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (1.094x ➖, 0↑ 1↓)
duckdb / parquet / ns (1.037x ➖, 0↑ 0↓)
|
commented
Oct 9, 2026
Benchmarks: Appian on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-compact / ns (0.997x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.996x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (0.990x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.004x ➖, 0↑ 0↓)
No file size changes detected. |
commented
Oct 9, 2026
Benchmarks: Clickbench on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.999x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (0.994x ➖, 1↑ 0↓)
datafusion / parquet / ns (0.999x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.013x ➖, 2↑ 4↓)
duckdb / vortex-compact / ns (1.012x ➖, 0↑ 2↓)
duckdb / parquet / ns (1.017x ➖, 1↑ 1↓)
No file size changes detected. |
commented
Oct 9, 2026
Benchmarks: TPC-DS SF=1 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.001x ➖, 0↑ 1↓)
datafusion / vortex-compact / ns (1.001x ➖, 0↑ 1↓)
datafusion / parquet / ns (1.002x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.994x ➖, 4↑ 2↓)
duckdb / vortex-compact / ns (1.001x ➖, 1↑ 1↓)
duckdb / parquet / ns (0.998x ➖, 2↑ 3↓)
No file size changes detected. |
commented
Oct 9, 2026
Benchmarks: TPC-H SF=10 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.002x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (0.992x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.996x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.993x ➖, 1↑ 0↓)
duckdb / vortex-compact / ns (0.995x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.996x ➖, 0↑ 0↓)
No file size changes detected. |
commented
Oct 9, 2026
Benchmarks: Statistical and Population Genetics 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.993x ➖, 0↑ 0↓)
datafusion / vortex-compact / ns (0.998x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.994x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.002x ➖, 0↑ 0↓)
duckdb / vortex-compact / ns (1.002x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.006x ➖, 0↑ 0↓)
No file size changes detected. |
commented
Oct 9, 2026
Benchmarks: Compression 📖Commits: PR vortex / vortex-file-compressed / ns (0.976x ➖, 0↑ 0↓)
vortex / vortex-file-compressed / bytes (1.000x ➖, 0↑ 0↓)
vortex / vortex-file-compressed / ratio (0.984x ➖, 0↑ 0↓)
vortex / parquet / ns (1.000x ➖, 0↑ 0↓)
vortex / parquet / bytes (1.000x ➖, 0↑ 0↓)
vortex / arrow-ipc / ns (0.987x ➖, 0↑ 0↓)
vortex / arrow-ipc / bytes (1.000x ➖, 0↑ 0↓)
|
Tests that built decimals with storage wider than their precision allows now use the bounded width, or a precision that admits the width they exercise. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015zQPnFWRi7CWXHpgBE8CiF
commented
Oct 9, 2026
Benchmarks: TPC-H SF=1 on S3 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.032x ➖, 0↑ 3↓)
datafusion / vortex-compact / ns (1.091x ➖, 1↑ 4↓)
datafusion / parquet / ns (0.973x ➖, 1↑ 0↓)
duckdb / vortex-file-compressed / ns (1.058x ➖, 0↑ 1↓)
duckdb / vortex-compact / ns (1.069x ➖, 0↑ 1↓)
duckdb / parquet / ns (1.011x ➖, 0↑ 0↓)
|
Benchmarks: Random Access (S3) 📖Commits: PR How to read Verdict and Engines
random-access / vortex-file-compressed / ns (1.024x ➖, 0↑ 1↓)
random-access / parquet / ns (1.019x ➖, 0↑ 0↓)
random-access / lance / ns (1.215x ➖, 0↑ 4↓)
|
commented
Oct 9, 2026
Benchmarks: TPC-H SF=10 on S3 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-compact / ns (1.075x ➖, 0↑ 2↓)
datafusion / parquet / ns (0.958x ➖, 1↑ 0↓)
duckdb / vortex-compact / ns (1.041x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.083x ➖, 0↑ 3↓)
|
Decimal byte parts written before storage was bounded by precision may be wider than it allows; host assembly narrows them explicitly while the GPU kernel, which cannot narrow device buffers, now validates instead of constructing unchecked. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015zQPnFWRi7CWXHpgBE8CiF
commented
Oct 9, 2026
Benchmarks: Random Access 📖Commits: PR How to read Verdict and Engines
vortex / arrow-ipc / ns (0.965x ➖, 2↑ 0↓)
random-access / vortex-file-compressed / ns (0.994x ➖, 0↑ 0↓)
random-access / parquet / ns (0.991x ➖, 0↑ 0↓)
random-access / lance / ns (0.983x ➖, 0↑ 0↓)
|
Byte-parts, sparse, row and fuzz tests that built decimals with storage wider than their precision allows now use the bounded width, or a precision that admits the width they exercise. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015zQPnFWRi7CWXHpgBE8CiF
The GPU filter test stores its i256 case at a precision that admits i256, and the Arrow export test for narrowing i256 storage below its precision is removed since such storage can no longer be constructed. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015zQPnFWRi7CWXHpgBE8CiF
commented
Oct 27, 2026
|
This PR has been marked as stale because it has been open for 14 days with no activity. Please comment or remove the stale label if you wish to keep it active, otherwise it will be closed in 7 days |
Resolve conflicts with the strict decimal storage bound: keep the checked constructors and explicit narrowing while adopting develop's test session statics, strict `DecimalType` decoding, and plain `from_arrow_decimal*` converters. Adapt develop's new tests that built decimal storage wider than their precision allows so each uses the storage its precision requires. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015zQPnFWRi7CWXHpgBE8CiF
Tests added on develop built decimal storage wider than their precision admits, which the checked constructors now reject. Give each the storage its precision requires, derive expected arrays in the result's own storage, and cap needle widths at the precision bound. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015zQPnFWRi7CWXHpgBE8CiF
The checked decimal constructors reject storage wider than the precision requires, so the compat fixtures that specified such a width can no longer build their written form. Keep the fixtures so their published files stay covered by `check`, which reads the wide form back through legacy narrowing, and write those columns at the canonical width instead. Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk> Signed-off-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015zQPnFWRi7CWXHpgBE8CiF
Decimal arrays currently permit physical storage wider than the smallest type required by their precision. Establish
values_type <= DecimalType::smallest_decimal_value_type(decimal_dtype)as the storage contract so compute kernels can rely on this upper bound.try_new,try_new_handle) rather than narrowing it; storage within the bound is accepted without copying.DecimalData::narrow_to_precisionandDecimalArray::try_new_narrowedfor importers that receive wider storage than the precision requires. Arrow decimal imports, legacy serialized arrays, andDecimalBytePartsassembly narrow explicitly through them; device buffers cannot be narrowed and are rejected. The GPU byte-parts kernel validates its storage instead of constructing unchecked.Stack 1/2: based on
develop. The dispatch macros and kernel migrations are in #10314, based on this PR.Validation:
cargo clippy -p vortex-array -p vortex-arrow --all-targets --all-features -- -D warnings,cargo fmt --all --checkwith the pinned nightly, and the tests ofvortex-array,vortex-arrow,vortex-row,vortex-sparse,vortex-fuzzandvortex-decimal-byte-partspass locally with the CI profile. CI is green on the current head, including the CUDA, sanitizer and miri jobs.Compatibility: constructing a decimal array with storage wider than its precision requires is now an error (a panic from the infallible
newconstructors). Arrow imports of low-precisionDecimal128/Decimal256columns allocate to narrow their storage. Legacy files with oversized decimal storage still decode, with narrowing on the host; on the GPU they are rejected. As before, constructors require non-null values to fit their declared precision; this change does not add per-value precision validation.