Skip to content

Commit 45f0a48

Browse files
feat(vortex-geo): null-propagating scalar functions
Make the binary geo scalar functions (ST_Distance, ST_Intersects, ST_Contains) null-propagating: a nullable geometry operand is allowed, and any row whose geometry input is null yields a null result (null in -> null out), matching SQL/OGC and the other Vortex binary kernels. - validate_geometry_operands no longer rejects nullable operands - return_dtype mirrors operand nullability; validity() uses the shared vortex_array::expr::union_child_validities (#8829); is_null_sensitive = false - shared execute module filters null rows before decoding, computes over the rows valid in both operands, and scatters results back under the combined mask - prune: query_aabb declines a null geometry literal instead of erroring - GeometryAabb::accumulate skips null rows so placeholders don't widen the box - tests for nullable columns, constant-null, column/column, empty input Signed-off-by: Nemo Yu <zyu379@wisc.edu>
1 parent f81df62 commit 45f0a48

13 files changed

Lines changed: 722 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: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,8 +202,18 @@ 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.
208+
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+
};
214+
// Null rows are gone, so every coordinate below belongs to a present geometry — the
215+
// `unmasked_field_by_name` reads are therefore safe. Min/max the raw x/y buffers directly:
216+
// cheap, and avoids `to_geometry`'s panic on empty points (which decoding would hit).
207217
let coords = flatten_coordinates(&array, ctx)?;
208218
let xs = coords
209219
.unmasked_field_by_name("x")?
@@ -254,6 +264,7 @@ mod tests {
254264
use crate::test_harness::multilinestring_column;
255265
use crate::test_harness::multipoint_column;
256266
use crate::test_harness::multipolygon_column;
267+
use crate::test_harness::nullable_point_column;
257268
use crate::test_harness::point_column;
258269
use crate::test_harness::polygon_column;
259270

@@ -304,6 +315,23 @@ mod tests {
304315
Ok(())
305316
}
306317

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