[connortsui20]

Summary

• Tracking Issue: Tracking Issue: Fix Canonical Decimal #10701

Every decimal leaf must now have the canonical width of its precision, which makes legacy vortex.decimal nodes at other widths invalid as leaves. Those nodes still exist: old files store a decimal at whatever width held its values, and WidenPlugin writes a decimal Widen as a legacy node, since vortex.widen has no wire form yet. Reading a legacy node into the canonical array or a decimal Widen keeps it readable.

Changes

The reader is a DecimalPlugin because a vtable's deserialize must return its own array type, while a plugin can return a Widen. It picks the child with WidenArray::from_stored_decimal, like DecimalByteParts decoding (#10905), so both paths give the same values the same precision. The plugin also owns the serializer, so the Decimal vtable has no serde, as with FoR. The plugin checks the wire layout first, so a malformed node returns an error instead of panicking.

Known cost: each legacy read scans its valid values once, because stored statistics are attached only after the plugin returns. This also applies to IPC and writes without a compressor, not only to old files.

Choosing the child precision of a legacy node scans its valid values on the CPU, so a legacy node in device memory now returns an error instead of being copied to the host. Known regression: a CUDA read of a legacy node, which worked before, fails until the CUDA executor reads legacy decimal nodes itself.

Follow-up: a legacy node can now read as a Widen, which has no Python subclass.

API Changes

The default session registers the new public DecimalPlugin in place of Decimal. The Decimal vtable no longer serializes or deserializes, so a session that registers Decimal directly cannot read or write decimal arrays.

[codspeed]

Merging this PR will degrade performance by 18.08%

2 warnings

⚠️ 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

⚠️ 1 benchmark is 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

❌ 1 regressed benchmark
✅ 1913 untouched benchmarks
⏩ 47 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
❌	CPU Simulation	missing_stats[16]	75.8 µs	92.6 µs	-18.08%

Tip

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

---

_{Comparing ct/decimal-read-plugin (0dcf448) with develop (f0b14cd)}

Footnotes

1. 47 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. ↩

[mhk197]

IIUC, i dont understand why this isn't decimal.v2 or similar since we want to enforce a new invariant? would also simplify plugin logic

[robert3005]

I think if we can avoid introducing new serialized ids we should, this maximizes compatibility. The old decimal let you do whatever you wanted - this is tightening of the bounds so old readers can always read the current values and new readers use Widen to get the desired behaviour

[mhk197]

This LGTM now but would get someone with CUDA integration experience to review the CUDA part?

[connortsui20]

oh wait I can make a cosmetic change first (store len in the stored decimal struct so we dont recalculate it / keep it close to the rest of the metadata)

[robert3005]

just remove these tests and the copy to host logic - we need to add handling for this in cuda executor not here