Skip to content

Commit 2ea51cb

Browse files
feat(vortex-geo): null-propagating scalar functions (#8803)
## What Make the binary geo scalar functions (`ST_Distance`, `ST_Intersects`, `ST_Contains`) null-propagating. A nullable geometry operand is now allowed, and any row whose geometry input is null yields a `NULL` result. ## Why Spiral needs the geo predicates to accept nullable inputs to run SpatialBench. ## How A geo kernel decodes each operand into a `geo_types` geometry, and a null row has no geometry to decode, so it can't compute over every row and mask the nulls afterwards the way the numeric kernels do. Instead, the shared `execute_null_propagating` dispatch drops the null rows, computes only the rows valid in both operands, and scatters the results back under the combined null mask. `return_dtype` is nullable iff an operand is, and `validate_geometry_operands` no longer rejects nullable operands. --------- Signed-off-by: Nemo Yu <zyu379@wisc.edu>
1 parent 81c8f13 commit 2ea51cb

13 files changed

Lines changed: 715 additions & 172 deletions

File tree

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

vortex-geo/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ geoarrow-cast = { workspace = true }
2424
prost = { workspace = true }
2525
vortex-array = { workspace = true }
2626
vortex-arrow = { workspace = true }
27+
vortex-buffer = { workspace = true }
2728
vortex-error = { workspace = true }
29+
vortex-mask = { workspace = true }
2830
vortex-session = { workspace = true }
2931
wkb = { workspace = true }
3032

vortex-geo/src/aggregate_fn/aabb.rs

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,8 +202,20 @@ impl AggregateFnVTable for GeometryAabb {
202202
Columnar::Canonical(canonical) => canonical.clone().into_array(),
203203
Columnar::Constant(constant) => constant.clone().into_array(),
204204
};
205-
// Min/max the raw x/y buffers directly - cheap, and avoids `to_geometry`'s panic on empty
206-
// points (which decoding each geometry would hit).
205+
// Drop null rows before reading coordinates: a null geometry's storage holds placeholder
206+
// coordinates (e.g. `(0, 0)`) that would otherwise widen the zone box and drag it toward
207+
// the origin. `filter` collapses an all-true mask back to the input, so a null-free batch
208+
// passes through unchanged.
209+
//
210+
// TODO(perf): on nullable data this `filter` compacts the whole column even for a single
211+
// null before we min/max it. A validity-aware min/max straight over the raw x/y buffers
212+
// would skip that copy. Left as-is for now: this is a write-time zone stat, and the common
213+
// non-nullable case already costs nothing (the all-true mask makes `filter` a no-op).
214+
let valid = array.validity()?.execute_mask(array.len(), ctx)?;
215+
let array = array.filter(valid)?;
216+
// Null rows are gone, so every coordinate below belongs to a present geometry — the
217+
// `unmasked_field_by_name` reads are therefore safe. Min/max the raw x/y buffers directly:
218+
// cheap, and avoids `to_geometry`'s panic on empty points (which decoding would hit).
207219
let coords = flatten_coordinates(&array, ctx)?;
208220
let xs = coords
209221
.unmasked_field_by_name("x")?
@@ -254,6 +266,7 @@ mod tests {
254266
use crate::test_harness::multilinestring_column;
255267
use crate::test_harness::multipoint_column;
256268
use crate::test_harness::multipolygon_column;
269+
use crate::test_harness::nullable_point_column;
257270
use crate::test_harness::point_column;
258271
use crate::test_harness::polygon_column;
259272

@@ -304,6 +317,23 @@ mod tests {
304317
Ok(())
305318
}
306319

320+
/// Null rows contribute no extent: their placeholder coordinates must not widen the box toward
321+
/// the origin (matching min/max, which skip nulls).
322+
#[test]
323+
fn null_rows_do_not_widen_aabb() -> VortexResult<()> {
324+
let session = vortex_array::array_session();
325+
let mut ctx = session.create_execution_ctx();
326+
327+
// The valid points sit far from the origin; the null row's storage placeholder is (0, 0).
328+
let column = nullable_point_column(vec![Some((5.0, 6.0)), None, Some((7.0, 8.0))])?;
329+
let mut acc = Accumulator::try_new(GeometryAabb, EmptyOptions, column.dtype().clone())?;
330+
acc.accumulate(&column, &mut ctx)?;
331+
332+
// The box covers only the two valid points, never (0, 0).
333+
assert_eq!(aabb(&acc.finish()?)?, (5.0, 6.0, 7.0, 8.0));
334+
Ok(())
335+
}
336+
307337
/// The AABB of a Polygon column unions every ring vertex - exercising the `List<List<Struct>>`
308338
/// unwrap, not just the bare Point struct.
309339
#[test]

vortex-geo/src/extension/mod.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,18 +72,17 @@ pub(crate) fn is_native_geometry(dtype: &DType) -> bool {
7272
})
7373
}
7474

75-
/// Validate the operands of a geo scalar function: each must be a native geometry type (so the
76-
/// kernel can decode it) and non-nullable (geometry arrays never carry nulls).
75+
/// Validate the operands of a geo scalar function: each must be a native geometry type so the
76+
/// kernel can decode it. The two operands need not share a geometry type — e.g. a `Point` against
77+
/// a `Polygon` is valid, since distance/containment/intersection across types is meaningful.
78+
/// Nullable operands are allowed; the kernels propagate nulls (a null geometry input yields a null
79+
/// result) rather than decoding null rows.
7780
pub(crate) fn validate_geometry_operands(dtypes: &[DType]) -> VortexResult<()> {
7881
for dtype in dtypes {
7982
vortex_ensure!(
8083
is_native_geometry(dtype),
8184
"geo: operand {dtype} is not a native geometry type"
8285
);
83-
vortex_ensure!(
84-
!dtype.is_nullable(),
85-
"geo: nullable operand {dtype} is unsupported"
86-
);
8786
}
8887
Ok(())
8988
}

vortex-geo/src/prune/distance.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,22 @@ mod tests {
175175
GeoDistancePrune.falsify(&predicate, &StatsRewriteCtx::new(&session, &scope))
176176
}
177177

178+
/// A null geometry literal (`ST_Distance(geom, NULL) <= r`) declines cleanly instead of
179+
/// erroring in the stats rewrite: the all-null predicate can never prune.
180+
#[test]
181+
fn null_literal_is_not_pruned() -> VortexResult<()> {
182+
let session = geo_session();
183+
184+
let scope = point_column(vec![0.0], vec![0.0])?.dtype().clone();
185+
let null_query = Scalar::null(scope.as_nullable());
186+
let distance = GeoDistance.new_expr(EmptyOptions, [root(), lit(null_query)]);
187+
let predicate = Binary.new_expr(Operator::Lte, [distance, lit(0.5f64)]);
188+
189+
let ctx = StatsRewriteCtx::new(&session, &scope);
190+
assert!(GeoDistancePrune.falsify(&predicate, &ctx)?.is_none());
191+
Ok(())
192+
}
193+
178194
/// All four distance comparisons prune (`<=`/`<` via min-distance, `>=`/`>` via max-distance);
179195
/// `==`/`!=` are left to the scan.
180196
#[rstest]

vortex-geo/src/prune/intersects.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ mod tests {
5959
use vortex_array::expr::Expression;
6060
use vortex_array::expr::lit;
6161
use vortex_array::expr::root;
62+
use vortex_array::scalar::Scalar;
6263
use vortex_array::scalar_fn::EmptyOptions;
6364
use vortex_array::scalar_fn::ScalarFnVTableExt;
6465
use vortex_array::stats::rewrite::StatsRewriteCtx;
@@ -114,6 +115,21 @@ mod tests {
114115
Ok(())
115116
}
116117

118+
/// A null geometry literal (`ST_Intersects(geom, NULL)`) declines cleanly instead of erroring
119+
/// in the stats rewrite: the all-null predicate can never prune.
120+
#[test]
121+
fn null_literal_is_not_pruned() -> VortexResult<()> {
122+
let session = geo_session();
123+
124+
let scope = point_column(vec![0.0], vec![0.0])?.dtype().clone();
125+
let null_query = Scalar::null(scope.as_nullable());
126+
let predicate = GeoIntersects.new_expr(EmptyOptions, [root(), lit(null_query)]);
127+
128+
let ctx = StatsRewriteCtx::new(&session, &scope);
129+
assert!(GeoIntersectsPrune.falsify(&predicate, &ctx)?.is_none());
130+
Ok(())
131+
}
132+
117133
/// End-to-end: a zone strictly separated from the query is skipped; zones containing or merely
118134
/// touching the query must scan, touching geometries intersect under OGC semantics.
119135
#[test]

vortex-geo/src/prune/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,10 @@ fn geometry_and_constant<'a>(
8282
/// Prove claims against this box rather than the constant itself: the constant lies inside it,
8383
/// so whatever holds for the box holds for the geometry.
8484
fn query_aabb(constant: &Scalar, ctx: &StatsRewriteCtx<'_>) -> VortexResult<Option<GeoRect<f64>>> {
85+
// A null geometry literal has no extent to prove against, so it can never prune.
86+
if constant.is_null() {
87+
return Ok(None);
88+
}
8589
// Decoding the constant into a concrete geometry runs through the compute stack, which needs
8690
// an execution context.
8791
let mut exec = ctx.session().create_execution_ctx();

0 commit comments

Comments
 (0)