[ump45nose]

Summary

• Tracking Issue: Decimal scalars panic on a stored value outside the precision #10749

DecimalArray::scalar_at and the decimal min_max aggregate panicked when a stored value did not fit the precision of the dtype, because both built their results with Scalar::decimal, which calls vortex_expect on the precision check. Since DecimalArray construction doesn't check its values and file reads trust the stored precision, a file holding such a value could make a read panic. With this change both calls return an error instead.

Still open: the panic is still reachable through cached stats. min_max returns a cached Stat::Min/Stat::Max before it reaches the kernel, and decimal stats decoded from a file are not checked against the precision, so an out-of-range stored stat still panics in stat_scalar. That needs a check at the read boundary and is tracked in #10807. The other Scalar::decimal callers on compute paths (sum, mean, cast, fill_null, between, the DecimalByteParts kernels) are also left for follow-ups.

Changes

• Add Scalar::try_decimal, a fallible version of Scalar::decimal. Scalar::decimal now calls it and still panics on a bad value, with the panic documented.
• DecimalArray::scalar_at uses Scalar::try_decimal and returns the error.
• The decimal min_max aggregate uses Scalar::try_decimal for the min and max and returns the error.
• DecimalByteParts::scalar_at uses Scalar::try_decimal instead of building the scalar by hand with Scalar::try_new.
• The fuzz scalar_at reference uses Scalar::try_decimal(..)? to match the implementation. Its callers still vortex_expect the result, so the harness still panics on such an input, which is intended: random_scalar only generates in-range decimals, so an out-of-range value there means an encoding produced bad data.
• vortex.scalar(100, dtype=decimal(2, 0)) in the Python bindings now raises ValueError instead of PanicException.
• The safety docs of DecimalArray::new_unchecked / new_unchecked_handle / new_unchecked_from_byte_buffer no longer list "values fit the precision" as a caller invariant. They now say values are not checked on construction or file reads, and that code turning stored values into scalars must use a fallible path. This matches what try_new actually checks.
• Tests: test_scalar_at_value_outside_precision_errors and test_decimal_value_outside_precision_errors match on the does not fit in precision message, return VortexResult<()>, and use #[rstest] cases (one element, min/max, and a negative min of -100i8). New nullable tests put 100i8 in a null slot of a decimal(2, 0) array with [5, 100] and check that min_max returns min = max = 5 and that scalar_at on the null index returns a null scalar.

Tested with cargo test -p vortex-array --lib (4433 passed, 1 ignored), cargo test -p vortex-decimal-byte-parts --lib (288 passed), cargo clippy -p vortex-array -p vortex-decimal-byte-parts --all-targets --all-features -- -D warnings, and rustfmt +nightly-2026-09-10 --check on the changed files. I could not build vortex-fuzz or vortex-python locally because custom-labels needs libclang for bindgen on my machine, so those two small changes rely on CI.

AI assistance: I used an AI coding assistant to help write this change and the description; I reviewed the code and ran the tests above myself.

[codspeed]

Merging this PR will degrade performance by 3.57%

3 warnings, 1 note

⚠️ Walltime results may be unreliable

Some benchmarks used the Walltime instrument outside CodSpeed Macro Runners, which can make the results inconsistent.

Learn more about Macro Runners

See affected benchmarks

⚠️ Unable to measure the execution of 2 benchmarks

These benchmarks measured no execution time, so their results are not comparable and count as unchanged. This usually happens when the compiler removes the code under test.

Prevent compiler optimizations

See affected benchmarks

⚠️ 22 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

ℹ️ 40 benchmarks have no flamegraph

The flamegraph wasn't generated: the symbols of the profiling trace could not be resolved, because no information about the language frames was collected.

See affected benchmarks

⚡ 4 improved benchmarks
❌ 2 regressed benchmarks
✅ 2402 untouched benchmarks
⏩ 12 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
❌	Walltime	mul_u64_nonnull_neon	20.5 µs	39.1 µs	-47.63%
❌	Walltime	deferred_bool_avx2[16384, (ConstantLhs, NullOnlyFailure)]	285.4 µs	347.9 µs	-17.97%
⚡	CPU Simulation	chunked_dict_primitive_canonical_into[f64, (1000, 1000, 10)]	203.3 µs	147.8 µs	+37.49%
⚡	Walltime	bitpack_blocked_compress_avx2	7.6 µs	6.7 µs	+12.21%
⚡	Walltime	add_constant_shapes_neon[(32768, PerRowNullableConstant)]	16.6 µs	15.1 µs	+10.23%
⚡	CPU Simulation	chunked_canonicalize_into[(20, 100, 16, 4)]	370.3 µs	336.5 µs	+10.04%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing ump45nose:fix/10749-decimal-scalar-precision (0ee4df9) with develop (0291778)}

Footnotes

1. 12 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[connortsui20]

Thanks for picking this up! The two changed call sites look correct. scalar_at only runs on valid rows (probe_scalar checks validity first), min_max filters out null slots, and checking only the min and max is enough because precision bounds are symmetric.

The panic from #10749 is still reachable through cached stats. min_max returns cached Stat::Min/Stat::Max before it reaches compute_min_max (min_max/mod.rs:90). Those stats are built by stat_scalar (stats/stats_set.rs:241), which calls Scalar::validate(..).vortex_expect(..). Decimal stats decoded from a file go through bytes_from_proto (scalar/proto.rs:406), which only checks the byte length, not the precision. So a file with a decimal(2, 0) column and a stored exact max of 100i8 still panics in min_max, and in any other stats consumer.

That points at the root cause: file reads trust the stored precision for both values and stats. Checking at that boundary (stats decode in StatsSet::from_flatbuffer, plus a value check in the decimal deserialize or the read plugin planned in #10701) would close the whole class instead of one call site at a time. If that's too much for this PR, it's fine to keep the scope here, but please say in the description that the stats path is still open and link a follow-up issue.

Smaller follow-ups that fit this PR:

• DecimalByteParts::scalar_at (encodings/decimal-byte-parts/src/decimal_byte_parts/array.rs:400) builds the same fallible scalar by hand with Scalar::try_new. It can call Scalar::try_decimal so every fallible decimal scalar goes through one helper.
• The fuzz reference in fuzz/src/array/scalar_at.rs:51 still uses Scalar::decimal. On the input this PR fixes, the reference now panics where the implementation returns an error, so the fuzzer reports a harness crash instead of comparing the two. It can use Scalar::try_decimal(..)?.
• vortex-python/src/scalar/factory.rs still calls Scalar::decimal on user input, so vortex.scalar(100, dtype=decimal(2, 0)) raises a PanicException instead of a ValueError. The FFI constructors already return an error for this case.

The inline comments cover the safety-contract question and the tests.

---

Generated by Claude Code

[ump45nose]

Thanks for the detailed review. Pushed 0ee4df9:

• Kept the scope of this PR and opened Decimal stats read from a file are not checked against the precision #10807 for the cached-stats path (stats decoded from a file aren't checked against the precision). The description now says that path is still open and links it.
• Picked the "values may fall outside the precision" contract: the safety docs on new_unchecked, new_unchecked_handle, and new_unchecked_from_byte_buffer no longer list it as a caller invariant and say values aren't checked on construction or file reads. Checking on deserialize can then come with Tracking Issue: Fix Canonical Decimal #10701 without these tests relying on a contract they break.
• Tests now match does not fit in precision, return VortexResult<()>, and use #[rstest] cases, including a negative min. Added the nullable [5, 100] / [true, false] case for both min_max (min = max = 5) and scalar_at (null scalar at index 1).
• DecimalByteParts::scalar_at, the fuzz scalar_at reference, and the Python scalar factory now go through Scalar::try_decimal; Python raises ValueError.

I couldn't build vortex-fuzz / vortex-python locally (bindgen needs libclang here), so those two rely on CI.

[connortsui20]

Thanks, this covers everything from the last round. The tests now reach the precision guard, the null-slot cases are covered, and the DecimalByteParts and Python changes look right (ValueError is a better fit than the default RuntimeError mapping for bad user input).

Two things left, both inline: the new contract docs on DecimalData::new_unchecked decide a question that's still open, and the fuzz claim in the description doesn't hold. The rest is a nit.

Keeping #10807 separate is the right call. It's one case of a broader question that Vortex hasn't settled yet: which parts of a file are validated on read and which are trusted (#10756 and the width decoding in #10755 are others). Whichever way that goes, bad data from a file should return an error and not panic, which is what this PR does for these call sites.

---

Generated by Claude Code

[connortsui20]

This is a style and structure pass on the full diff at 0ee4df9. The open threads from the last round still apply. Nothing here changes behavior.

• array.rs: the # Safety sections on DecimalData::new_unchecked and new_unchecked_handle now list only a validity duty for a parameter neither function takes. They also leave out the length and alignment checks that validate actually performs.
• operations.rs: the "stored values are not guaranteed to fit" comment is copied in two places, and it restates the contract owned by DecimalData::new_unchecked.
• min_max/decimal.rs: with try_decimal, compute_min_max reads better as a guard plus one constructor pair. The caller doesn't need its Ok(match .. ?) wrapper.
• One nit on the vortex_expect message.

The tests follow the patterns already in their modules, so I have no further comments on them.

---

Generated by Claude Code