Skip to content

Commit 9ad9376

Browse files
refactor(vortex-geo): move null-propagating dispatch to its own module
Address PR review round 2: - Move the shared null-propagating dispatch out of scalar_fn/mod.rs into scalar_fn/null_propagate.rs, keeping mod.rs to module-tree wiring. - Bring ST_Distance and ST_Intersects null-path coverage to parity with ST_Contains: all-null-column and empty-combined-mask early returns, plus the two-column path for ST_Intersects. Signed-off-by: Nemo Yu <zyu379@wisc.edu>
1 parent 0716c2c commit 9ad9376

5 files changed

Lines changed: 371 additions & 265 deletions

File tree

vortex-geo/src/scalar_fn/contains.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ use vortex_session::VortexSession;
2222
use vortex_session::registry::CachedId;
2323

2424
use crate::extension::validate_geometry_operands;
25-
use crate::scalar_fn::binary_result_validity;
26-
use crate::scalar_fn::execute_null_propagating;
25+
use crate::scalar_fn::null_propagate::binary_result_validity;
26+
use crate::scalar_fn::null_propagate::execute_null_propagating;
2727

2828
/// OGC `ST_Contains` between two native geometry operands, each a column or a constant
2929
/// literal: true where operand `b` lies completely inside operand `a` (boundary contact alone

vortex-geo/src/scalar_fn/distance.rs

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ use vortex_session::VortexSession;
2424
use vortex_session::registry::CachedId;
2525

2626
use crate::extension::validate_geometry_operands;
27-
use crate::scalar_fn::binary_result_validity;
28-
use crate::scalar_fn::execute_null_propagating;
27+
use crate::scalar_fn::null_propagate::binary_result_validity;
28+
use crate::scalar_fn::null_propagate::execute_null_propagating;
2929

3030
/// Planar (Euclidean) `ST_Distance` (no geodesic correction) between two native geometry
3131
/// operands, each a column or a constant literal.
@@ -268,6 +268,38 @@ mod tests {
268268
Ok(())
269269
}
270270

271+
/// An entirely-null geometry column yields an all-null output (the fully-null fast path in
272+
/// `eval_column`).
273+
#[test]
274+
fn distance_all_null_column_is_all_null() -> VortexResult<()> {
275+
let session = vortex_array::array_session();
276+
let mut ctx = session.create_execution_ctx();
277+
278+
let a = nullable_point_column(vec![None, None])?;
279+
let b = point_constant(0.0, 0.0, 2, &mut ctx)?;
280+
let distance = GeoDistance::try_new_array(a, b)?.into_array();
281+
282+
let expected = PrimitiveArray::new(vec![0.0f64; 2], Validity::AllInvalid).into_array();
283+
assert_arrays_eq!(distance, expected, &mut ctx);
284+
Ok(())
285+
}
286+
287+
/// Two nullable columns whose nulls never line up: the combined mask is empty, so the output
288+
/// is all null (the `valid.all_false()` early return in `eval_column_pair`).
289+
#[test]
290+
fn distance_column_pair_all_null() -> VortexResult<()> {
291+
let session = vortex_array::array_session();
292+
let mut ctx = session.create_execution_ctx();
293+
294+
let a = nullable_point_column(vec![Some((0.0, 0.0)), None])?;
295+
let b = nullable_point_column(vec![None, Some((1.0, 1.0))])?;
296+
let distance = GeoDistance::try_new_array(a, b)?.into_array();
297+
298+
let expected = PrimitiveArray::new(vec![0.0f64; 2], Validity::AllInvalid).into_array();
299+
assert_arrays_eq!(distance, expected, &mut ctx);
300+
Ok(())
301+
}
302+
271303
/// A non-geometry operand dtype is rejected up front, before execution.
272304
#[test]
273305
fn non_geometry_operand_is_rejected() -> VortexResult<()> {

vortex-geo/src/scalar_fn/intersects.rs

Lines changed: 66 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ use vortex_session::VortexSession;
2222
use vortex_session::registry::CachedId;
2323

2424
use crate::extension::validate_geometry_operands;
25-
use crate::scalar_fn::binary_result_validity;
26-
use crate::scalar_fn::execute_null_propagating;
25+
use crate::scalar_fn::null_propagate::binary_result_validity;
26+
use crate::scalar_fn::null_propagate::execute_null_propagating;
2727

2828
/// OGC `ST_Intersects` (not disjoint; boundary contact counts) between two native geometry
2929
/// operands, each a column or a constant literal.
@@ -337,6 +337,70 @@ mod tests {
337337
Ok(())
338338
}
339339

340+
/// Both operands nullable columns: the verdict is null wherever either row is null, and
341+
/// computed on the rows valid in both (points intersect exactly when equal).
342+
#[test]
343+
fn intersects_propagates_column_pair_nulls() -> VortexResult<()> {
344+
let session = vortex_array::array_session();
345+
let mut ctx = session.create_execution_ctx();
346+
347+
let a = nullable_point_column(vec![
348+
Some((0.0, 0.0)),
349+
None,
350+
Some((2.0, 2.0)),
351+
Some((3.0, 3.0)),
352+
])?;
353+
let b = nullable_point_column(vec![
354+
Some((0.0, 0.0)),
355+
Some((5.0, 5.0)),
356+
None,
357+
Some((9.0, 9.0)),
358+
])?;
359+
let intersects = GeoIntersects::try_new_array(a, b)?.into_array();
360+
361+
let expected = BoolArray::new(
362+
BitBuffer::from_iter([true, false, false, false]),
363+
Validity::from_iter([true, false, false, true]),
364+
)
365+
.into_array();
366+
assert_arrays_eq!(intersects, expected, &mut ctx);
367+
Ok(())
368+
}
369+
370+
/// An entirely-null geometry column yields an all-null output (the fully-null fast path in
371+
/// `eval_column`).
372+
#[test]
373+
fn intersects_all_null_column_is_all_null() -> VortexResult<()> {
374+
let session = vortex_array::array_session();
375+
let mut ctx = session.create_execution_ctx();
376+
377+
let points = nullable_point_column(vec![None, None])?;
378+
let query = geometry_constant(&donut(), 2)?;
379+
let intersects = GeoIntersects::try_new_array(points, query)?.into_array();
380+
381+
let expected =
382+
BoolArray::new(BitBuffer::from_iter([false, false]), Validity::AllInvalid).into_array();
383+
assert_arrays_eq!(intersects, expected, &mut ctx);
384+
Ok(())
385+
}
386+
387+
/// Two nullable columns whose nulls never line up: the combined mask is empty, so the output
388+
/// is all null (the `valid.all_false()` early return in `eval_column_pair`).
389+
#[test]
390+
fn intersects_column_pair_all_null() -> VortexResult<()> {
391+
let session = vortex_array::array_session();
392+
let mut ctx = session.create_execution_ctx();
393+
394+
let a = nullable_point_column(vec![Some((0.0, 0.0)), None])?;
395+
let b = nullable_point_column(vec![None, Some((1.0, 1.0))])?;
396+
let intersects = GeoIntersects::try_new_array(a, b)?.into_array();
397+
398+
let expected =
399+
BoolArray::new(BitBuffer::from_iter([false, false]), Validity::AllInvalid).into_array();
400+
assert_arrays_eq!(intersects, expected, &mut ctx);
401+
Ok(())
402+
}
403+
340404
/// A non-geometry operand dtype is rejected up front, before execution.
341405
#[test]
342406
fn non_geometry_operand_is_rejected() -> VortexResult<()> {

0 commit comments

Comments
 (0)