## Summary
FoR's `CastReduce` now pushes down only casts that change nullability
(`eq_ignore_nullability`). Every cast that changes the integer type
declines, so the array decodes and the decoded values are cast. This
fixes three correctness bugs in casting FoR arrays. Two of them have
been there since before the new FoR array.
## Background
FoR stores each value as an offset from a reference: `encoded =
value.wrapping_sub(reference)` in the source type, and decoding is
`encoded.wrapping_add(reference)` in whatever type the array has. The
encoded values are therefore offsets modulo 2ⁿ, stored in the source
type, and not ordinary values.
The old kernel cast the encoded child and the references to the target
type separately, then rebuilt the FoR array. That treats the offsets as
plain values, which is only correct when every offset fits the source
type as a non-negative number and every decoded value fits the target
type.
## Bugs
Reproduced on `develop` and, for the first two, on the commit before the
blocked FoR stack (`ba5fcfcde0^`):
| Cast | Expected | Got |
|---|---|---|
| `[-128i8, 127]` → `i16` | `[-128, 127]` | `[-128, -129]` |
| `[100i16, 200]` → `i8` | `Err(value exceeds target range)` | `[100,
-56]` |
| `5..10` (reference `-5`) → `u32` | `[5, 6, 7, 8, 9]` | `Err(No
CastReduce to cast constant array from i32 to u32)` on decode |
1. **Widening a signed type whose range exceeds `T::MAX` gives wrong
values.** `[-128i8, 127]` has reference `-128`, so `127` is stored as
`255`, which wraps to `-1` in `i8`. Casting the encoded child to `i16`
sign-extends it to `-1`, which decodes as `-128 + -1 = -129`. Over `i8`
values `-128..=127`, 128 of 256 values come out wrong, with no error.
`FoR::encode` produces this whenever a signed column's range exceeds the
type's maximum.
2. **Narrowing wraps instead of erroring.** In `[100i16, 200]` cast to
`i8`, the reference `100` and the offsets `0` and `100` each fit `i8`,
but `100 + 100` doesn't, and the wrapping decode hides the overflow.
Sign changes such as `i8` → `u8` and `u8` → `i8` fail the same way. The
opposite also happens: `-100i32..100` fits `i8`, but its offsets
`0..=199` don't, so the cast fails on decode.
3. **An out-of-range reference leaves an undecodable array.** Values
`5..10` with reference `-5` can remain after filtering away the
negatives. Cast to `u32`, the values fit but the reference doesn't.
`Constant`'s `CastReduce` declines, so the references slot is left
holding a lazy `vortex.cast`, which `try_new_chunked` accepts because it
only checks dtype and length. The cast appears to succeed, and decoding
fails. Before #10105, the scalar reference cast failed eagerly, so the
whole cast errored instead. Per-chunk references hit the same issue,
because a slice keeps the references of every chunk it overlaps.
The existing tests didn't catch these: every conformance case has a
small range and widens.
## Why decline instead of proving the cast safe
- **The pushdown doesn't save work.** BitPacked's `CastReduce` only
handles nullability, and its widening `CastKernel` unpacks into a
full-width buffer. A pushed-down widening cast therefore executes as
`FoR(Cast(BitPacked))`: it unpacks into a wide buffer, then adds the
reference in a second pass, because the child is no longer BitPacked and
FoR's fused unpack-and-add path doesn't apply. Declining gives one fused
decode in the narrow type, then a widening primitive cast. The pass
count is the same, with the fused pass over narrower data.
- **Nothing downstream benefits.** For example, a compare on
`FoR(Cast(BitPacked))` can't use BitPacked's compare kernel.
- **It wouldn't apply to per-chunk references.** Since #10136,
`FoRScheme` writes per-chunk references when `fastlanes.for.v2` is
allowed. The proof needs the minimum and maximum of the references
child, which may be lazy inside a reduce rule, so every type change
would decline there anyway.
A nullability-only cast is always safe: it changes neither the values
nor the non-nullable references, so it is still pushed down, for single
and per-chunk references alike.
---------
Signed-off-by: Matt Katz <[email protected]>
Stacked on #10135.
Summary
FoRSchemecompresses with one reference per 1024-element chunk when the compressor's allowed serialized IDs includefastlanes.for.v2, followingDecimalScheme. Default writes are unchanged: the core edition allows onlyfastlanes.for, sorefinekeeps v1.FoRSchemeFoRSchemegains v1 and v2 modes withv1()andv2()constructors;Defaultis v1. The session registersFOR_V1.refinereturns v2 whenfastlanes.for.v2is allowed, and v1 otherwise.produced_encodings: v1 declaresfastlanes.for; v2 declaresfastlanes.forandfastlanes.for.v2.FoR::encode_chunked. Per-chunk references never pack wider than one reference, and the chunked encoder is faster than the single-reference one. The encoded child is bit-packed as before, and the references are compressed as child 0 withcompress_child. When every chunk has the same minimum, the references compress to a constant and the array serializes asfastlanes.for.CUDA
The CUDA preset excludes
fastlanes.for.v2, as it does multi-part decimals. The GPU FoR decoder returns an error for per-chunk references.Follow-ups
encode_chunkedbecause it writes nothing. The ratio should then count the references child. Done whenestimate_skips_arrays_only_chunk_references_narrowexpects v2, the golden snapshots don't regress, and the change in compression time is measured.