Skip to content

Commit eea991e

Browse files
refactor(vortex-geo): drop redundant all-true filter guards
ArrayRef::filter collapses an all-true mask back to the input (Filter's reduce rule + optimize), so the if valid.all_true() guards in GeometryAabb::accumulate and the execute helpers were redundant. Filter unconditionally; all-valid inputs pass through unchanged. Signed-off-by: Nemo Yu <zyu379@wisc.edu>
1 parent 45f0a48 commit eea991e

2 files changed

Lines changed: 10 additions & 22 deletions

File tree

vortex-geo/src/aggregate_fn/aabb.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -204,13 +204,10 @@ impl AggregateFnVTable for GeometryAabb {
204204
};
205205
// Drop null rows before reading coordinates: a null geometry's storage holds placeholder
206206
// coordinates (e.g. `(0, 0)`) that would otherwise widen the zone box and drag it toward
207-
// the origin.
207+
// the origin. `filter` collapses an all-true mask back to the input, so a null-free batch
208+
// passes through unchanged.
208209
let valid = array.validity()?.execute_mask(array.len(), ctx)?;
209-
let array = if valid.all_true() {
210-
array
211-
} else {
212-
array.filter(valid)?
213-
};
210+
let array = array.filter(valid)?;
214211
// Null rows are gone, so every coordinate below belongs to a present geometry — the
215212
// `unmasked_field_by_name` reads are therefore safe. Min/max the raw x/y buffers directly:
216213
// cheap, and avoids `to_geometry`'s panic on empty points (which decoding would hit).

vortex-geo/src/scalar_fn/execute.rs

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -188,13 +188,9 @@ where
188188
{
189189
let len = column.len();
190190
let valid = column.validity()?.execute_mask(len, ctx)?;
191-
// Decode only the non-null rows, since a null row has no geometry to decode. The common
192-
// all-valid case decodes the column directly and skips the filter.
193-
let decoded = if valid.all_true() {
194-
geometries(column, ctx)?
195-
} else {
196-
geometries(&column.filter(valid.clone())?, ctx)?
197-
};
191+
// Drop the null rows before decoding, since a null row has no geometry to decode. `filter`
192+
// collapses an all-true mask, so an all-valid column passes through unchanged.
193+
let decoded = geometries(&column.filter(valid.clone())?, ctx)?;
198194
let values = decoded.iter().map(f).collect();
199195
Ok(T::build_array(len, &valid, values, nullability))
200196
}
@@ -217,15 +213,10 @@ where
217213
let b_present = b.validity()?.execute_mask(len, ctx)?;
218214
// A row survives only where both columns are present.
219215
let valid = &a_present & &b_present;
220-
// Keep only the rows valid in both columns, so decoding never sees a null geometry. The
221-
// common all-valid case decodes the columns directly and skips the filter.
222-
let (a, b) = if valid.all_true() {
223-
(a.clone(), b.clone())
224-
} else {
225-
(a.filter(valid.clone())?, b.filter(valid.clone())?)
226-
};
227-
let ag = geometries(&a, ctx)?;
228-
let bg = geometries(&b, ctx)?;
216+
// Keep only the rows valid in both columns, so decoding never sees a null geometry. `filter`
217+
// collapses an all-true mask, so all-valid columns pass through unchanged.
218+
let ag = geometries(&a.filter(valid.clone())?, ctx)?;
219+
let bg = geometries(&b.filter(valid.clone())?, ctx)?;
229220
let values = ag.iter().zip(&bg).map(|(x, y)| compute(x, y)).collect();
230221
Ok(T::build_array(len, &valid, values, nullability))
231222
}

0 commit comments

Comments
 (0)