Repository navigation
Check validity before removing VarBinView nullability - #10267
connortsui20 wants to merge 1 commit into
Conversation
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Merging this PR will degrade performance by 48.14%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | bench_compare_sliced_dict_primitive[(2500, 10000)] |
42.8 µs | 82.4 µs | -48.14% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing ct/stats-nullability-proof (bb58423) with ct/stats-writer-cache-computation (aef451a)
Footnotes
-
503 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. ↩
Superseded by #10269, which groups this change with the related statistics work. The original commits and branch are preserved.
Original PR description
Tracking Issue: #10177
Summary
Removing nullability from a VarBinView can expose null-row bytes as UTF-8 or make previously hidden views accessible. A cached Boolean minimum is a semantic claim, so it cannot establish the validity required by that unchecked conversion. Incorrect claims can enter through a safe cache transfer, even when the original result was computed correctly for its own input.
Changes
Check the executed validity bits at the VarBinView cast's unchecked-construction boundary. Constant validity retains its existing shortcuts, and strings do not need another UTF-8 scan because every exposed row was already valid in the source. Other cast kernels retain their cached shortcuts.
Stack
Depends on #10266 in migration stack #10229.