Repository navigation
fix(runend): clip min/max and is_constant to the array's logical window - #9960
jackylee-ch wants to merge 1 commit into
Conversation
`RunEndData::validate_parts` only requires the runs to *cover* `offset..offset + length`, so a run-end array may legally carry runs that lie entirely outside its logical window. `RunEndMinMaxKernel` and `RunEndIsConstantKernel` aggregated over every entry of `values()`, so they saw values no row of the array holds. Measured against the decoded array: for `try_new_offset_length(ends=[7, 10], values=[2, 3], offset=2, length=3)` -- logical rows `[2, 2, 2]` -- `statistics().compute_stat(Stat::Max)` returned 3 where the decoded array returns 2. That is the path that fills a file's zone map, and DataFusion and DuckDB read those statistics as column statistics. The error is always outward, min too low and max too high, so pruning stays sound and no rows go missing; what is wrong is the statistic itself. The is_constant kernel is one-directional in the same way, reporting a constant window as non-constant. The sibling aggregate in this crate already clips: `RunEndSumKernel` takes `array.offset()..array.offset() + batch.len()` and `sum/runs.rs` trims the boundary runs to it. Share that treatment through a `windowed_values` helper, which returns `values()` untouched when the window spans every run -- the common case, since the slice kernel trims both children. Signed-off-by: jackylee-ch <qcsd2011@gmail.com>
|
This might happen but every time we perform operation on RunEnd array we trim the values. How do you manage to arrive at this where values has extraneous entries? |
|
Nothing in the workspace builds one. The slice kernel and the Arrow import trim both children, filter re-encodes at offset 0, and encode, BtrBlocks and the zone map build fresh arrays at offset 0; scalar-fn push-down, cast, fill_null, validity and take_from keep whatever shape they are given. The way in is
|
|
Closing this. Walking the kernels one at a time isn't the right shape: The clean fix is one invariant — have Nothing in the workspace builds an out-of-window array today: the slice kernel and the |
RunEndData::validate_partsonly requires the runs to coveroffset..offset + length— the lastrun end must be
>=offset+length, and the first>=offset — so a run-end array may legally carryruns that lie entirely outside its logical window.
RunEndMinMaxKernel(
encodings/runend/src/compute/min_max.rs:40) andRunEndIsConstantKernel(
compute/is_constant.rs:37) aggregated over every entry ofvalues(), so they saw values no row ofthe array holds.
Measured against the decoded array, with the fix reverted:
ends=[7,10] values=[2,3] offset=2 length=3[2,2,2]Stat::Maxends=[4,9] values=[2,3] offset=4 length=2[3,3]Stat::Minends=[3,6,9] values=[1,5,9] offset=3 length=2[5,5]Stat::Minstatistics().compute_stat(Stat::Min)/(Stat::Max)is the path that fills a file's zone map, andboth engine integrations surface those as column statistics
(
vortex-datafusion/src/convert/stats.rs,vortex-duckdb/src/column_statistics.rs).Scope, stated plainly: the error is always outward — min too low, max too high — so zone-map pruning
stays sound and no rows go missing. What is wrong is the statistic. The
is_constantkernel isone-directional in the same way: it reports a constant window as non-constant, costing constant
folding, and can never report a non-constant window as constant. Going through the
min_max()helper returns the correct answer on these arrays, so this is not a wrong
MIN()/MAX()in a query.Vortex's own slice kernel trims both children (
kernel.rs:79), so it does not produce this shape;try_new_offset_lengthand a file whose run-end metadata declares a narrower window do.Fix
RunEndSumKernelalready clips —sum/kernel.rs:79takesarray.offset()..array.offset() + batch.len()andsum/runs.rstrims the boundary runs to it. Thisgives min/max and is_constant the same treatment through a
windowed_valueshelper, which returnsvalues()untouched when the window already spans every run.Tests
cargo test --release -p vortex-runend: 130 passed, 126 before.The first version of these tests was hollow: comparing
min_max(&array)withmin_max(&decoded)passed with the clipping removed, because that helper does not reach this kernel for these arrays.
They now assert on
compute_statand driveRunEndMinMaxKernel::aggregatedirectly, the waycompute/sum/tests.rsdoes. Withwindowed_valuesneutered, three of the four cases fail;window_spans_every_runpasses either way, which is the fast path.One case had to be corrected against the validator:
ends=[2,6,9]withoffset=3is rejected byvalidate_partswith "First run end 2 must be >= offset 3", so a leading run can only sit outsidethe window when it ends exactly at the offset.
AI assistance
Written with agentic AI assistance; the non-hollowness probe is what caught both the hollow first
test and the fact that the reachable path is the statistics one rather than the aggregate helper,
and the value claim above was narrowed to match.