fix: propagate validity in RunEnd and Sparse lazy decoders (#225, #226)#231
Conversation
Two silent-null-loss bugs surfaced by Raincloud conformance triage round 4, both the same class as #210 (validity dropped when unwrapping a MaskedArray child). Fixed by re-wrapping the decoded result in a MaskedArray whose per-row validity mirrors the Rust reference vtables — expressed lazily, O(runs)/O(patches) not O(rows), since n can be huge (uci-online-retail has 499712 rows). RunEnd (#225): a nullable run-value dropped its mask, so a null run expanded to a filler value (uci-online-retail `customerid` u16?: null rows emitted the FoR base). Per Rust `ValidityVTable<RunEnd>`, a RunEnd array's validity IS a RunEnd over the same ends whose per-run value is the run-value's validity bit; a row's validity is thus the validity of the run it falls in. Implemented as `LazyRunEndBoolArray(ends, values-validity, offset)`. Sparse (#226), two facets. Null fill was coerced to 0 (`scalarToLong` returns 0 for a null scalar), so a `fill_value: null` array decoded every unpatched position as 0.0 (world-energy-consumption `biofuel_cons_change_pct` f64?). Nullable patch values were stripped, so a patched-but-null position decoded to raw 0 (nuclear_share_energy). Per Rust `ValidityVTable<Sparse>`, row validity is a sparse bool whose fill is `fill_value.is_valid()` and whose per-patch value is the patch value's validity bit — so a position is valid iff (it is a patch AND that patch is valid) OR (it is unpatched AND the fill is non-null). Null fill is detected from the ScalarValue (explicit null_value or no value-bearing field set), not from `scalarToLong`'s lossy 0. Corpus evidence: both slugs now match their Parquet oracle value-for-value (uci-online-retail 4335272 cells, world-energy-consumption 3039010 cells); expected-status.csv flips both gap entries to ok. Closes #225 Closes #226 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Reviewed (vortex-reviewer, built at ref + ran suites): APPROVE, no correctness/memory blocker. Validity propagation verified faithful to the Rust |
Round 5 results (tested against origin/main, 512cfbd): - bi-citymaxcapita: ok — 28.3M cells match (#231 fixed RunEnd null propagation) - bi-euro2016: gap:235 — DateTimeParts throws on null day cells (#231 exposed #235) - bi-hatred: ok - bi-yalelanguages: gap:235 — DateTimeParts throws on null day cells - covid-world-vaccination-progress: ok - hotel-booking-demand: ok (date32 temporal offset matches oracle) - movies: ok - uci-credit-approval: ok - uci-diabetes-130-us-hospitals: ok - uci-online-retail-ii: ok Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fixes the two round-4 silent-null-loss bugs (#205 triage), same class as the fixed #210: the RunEnd and Sparse lazy decoders unwrapped their children's
MaskedArrayto.inner()and dropped validity, so null rows decoded as plausible values.Semantics translated directly from the Rust
ValidityVTableimpls (encodings/runend/src/array.rs,encodings/sparse/src/lib.rs):LazyRunEndBoolArray), O(runs) — essential since n can be huge (uci-online-retail 499,712 rows); a per-row bitmap would defeat the lazy path.LazySparseBoolArray), O(patches). Two facets fixed: the null fill value (previouslyscalarToLongcoerced null→0, so unpatched positions rendered 0.0), now detected from theProtoScalarValueitself (a real 0 fill sets a typed field; null setsnull_value/none); and the stripped nullable patch_values mask.Validation: new
RunEndEncodingDecoderTest(3) +SparseEncodingDecoderTest(4) covering null-run/null-fill/null-patch and the no-regression non-nullable path; 2SparseEncodingEncoderTestround-trips updated (they asserted the old null-fill-as-zero behavior). Real corpus:uci-online-retail(4.3M cells) andworld-energy-consumption(3.0M cells) both match the Parquet oracle exactly. Matrix: bothgap:225/gap:226→ok(0 gaps). Full./mvnw verifygreen.Known follow-up (out of scope, filed separately): VarBin/Utf8 sparse (
decodeVarBin) still drops null-fill validity — same class, no corpus column hits it yet.Closes #225
Closes #226
🤖 Generated with Claude Code