Skip to content

Commit 488a97c

Browse files
fix(vortex-geo): decode null geometry rows as None in aabbs_2d
A null geometry matches nothing in a spatial join (the predicate is NULL under SQL three-valued logic), so aabbs_2d must give a null row the same slot as an empty geometry - None - instead of aborting the whole decode. Generalize the decode: the per-type decoders now surface geoarrow's per-row Option (a null row is None) through a shared geometry_row step, geometries_opt is the null-aware column decode, and the strict geometries() becomes a thin wrapper that errors on any None, preserving its behavior for the scalar functions and single_geometry, which reject nullable operands up front. The Option reflects the array's logical validity - pinned by a constant-null test, where no storage validity exists until canonicalization. Signed-off-by: Nemo Yu <zyu379@wisc.edu>
1 parent 5dbbc22 commit 488a97c

10 files changed

Lines changed: 200 additions & 77 deletions

File tree

vortex-geo/src/bounds.rs

Lines changed: 65 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,23 +8,24 @@ use vortex_array::ArrayRef;
88
use vortex_array::ExecutionCtx;
99
use vortex_error::VortexResult;
1010

11-
use crate::extension::geometries;
11+
use crate::extension::geometries_opt;
1212

1313
/// Per-row 2-D axis-aligned bounding boxes (AABBs) of a native geometry column, as `(min, max)`
1414
/// pairs of `[x, y]` corners.
1515
///
1616
/// The per-row counterpart of the [`GeometryAabb`](crate::aggregate_fn::GeometryAabb) aggregate,
17-
/// which unions the whole column into one box. A point yields a degenerate box with `min == max`;
18-
/// a geometry with no extent (e.g. an empty polygon) yields `None`.
17+
/// which unions the whole column into one box. A point yields a degenerate box with `min == max`.
18+
/// A null row, or a geometry with no extent (e.g. an empty polygon), yields `None`: either way
19+
/// the row has no box, and matches nothing in a spatial join.
1920
///
2021
/// A non-geometry column is an error. Any `z`/`m` ordinates carry no bounds, this decodes to 2-D `geo_types`.
2122
pub fn aabbs_2d(
2223
array: &ArrayRef,
2324
ctx: &mut ExecutionCtx,
2425
) -> VortexResult<impl Iterator<Item = Option<([f64; 2], [f64; 2])>> + use<>> {
25-
Ok(geometries(array, ctx)?.into_iter().map(|geometry| {
26+
Ok(geometries_opt(array, ctx)?.into_iter().map(|geometry| {
2627
geometry
27-
.bounding_rect()
28+
.and_then(|geometry| geometry.bounding_rect())
2829
.map(|rect| ([rect.min().x, rect.min().y], [rect.max().x, rect.max().y]))
2930
}))
3031
}
@@ -33,14 +34,18 @@ pub fn aabbs_2d(
3334
mod tests {
3435
use vortex_array::IntoArray;
3536
use vortex_array::VortexSessionExecute;
37+
use vortex_array::arrays::ConstantArray;
3638
use vortex_array::arrays::PrimitiveArray;
39+
use vortex_array::scalar::Scalar;
3740
use vortex_error::VortexResult;
3841

3942
use super::aabbs_2d;
4043
use crate::test_harness::linestring_column;
4144
use crate::test_harness::multilinestring_column;
4245
use crate::test_harness::multipoint_column;
4346
use crate::test_harness::multipolygon_column;
47+
use crate::test_harness::nullable_multipolygon_column;
48+
use crate::test_harness::nullable_point_column;
4449
use crate::test_harness::point_column;
4550
use crate::test_harness::polygon_column;
4651
use crate::test_harness::rect_column;
@@ -139,6 +144,61 @@ mod tests {
139144
Ok(())
140145
}
141146

147+
/// A null geometry row yields `None`, just like an empty geometry: no box, so it matches
148+
/// nothing in a spatial join. Valid rows keep their boxes.
149+
#[test]
150+
fn null_row_yields_none() -> VortexResult<()> {
151+
let session = vortex_array::array_session();
152+
let mut ctx = session.create_execution_ctx();
153+
154+
let points = nullable_point_column(vec![Some((1.0, 2.0)), None, Some((3.0, 4.0))])?;
155+
assert_eq!(
156+
aabbs_2d(&points, &mut ctx)?.collect::<Vec<_>>(),
157+
vec![bounds(1.0, 2.0, 1.0, 2.0), None, bounds(3.0, 4.0, 3.0, 4.0)]
158+
);
159+
Ok(())
160+
}
161+
162+
/// Valid, empty, and null rows stay positionally aligned: the valid row keeps its box, and
163+
/// the empty and null rows are both `None`.
164+
#[test]
165+
fn mixed_valid_empty_null_rows_align() -> VortexResult<()> {
166+
let session = vortex_array::array_session();
167+
let mut ctx = session.create_execution_ctx();
168+
169+
let multipolygons = nullable_multipolygon_column(vec![
170+
Some(vec![vec![vec![
171+
(0.0, 0.0),
172+
(1.0, 0.0),
173+
(1.0, 1.0),
174+
(0.0, 1.0),
175+
]]]),
176+
Some(vec![]),
177+
None,
178+
])?;
179+
assert_eq!(
180+
aabbs_2d(&multipolygons, &mut ctx)?.collect::<Vec<_>>(),
181+
vec![bounds(0.0, 0.0, 1.0, 1.0), None, None]
182+
);
183+
Ok(())
184+
}
185+
186+
/// A constant-null column has no materialized storage validity until canonicalization, so
187+
/// this pins that the decode reflects the array's logical validity: every row is `None`.
188+
#[test]
189+
fn constant_null_column_is_all_none() -> VortexResult<()> {
190+
let session = vortex_array::array_session();
191+
let mut ctx = session.create_execution_ctx();
192+
193+
let dtype = point_column(vec![0.0], vec![0.0])?.dtype().as_nullable();
194+
let nulls = ConstantArray::new(Scalar::null(dtype), 2).into_array();
195+
assert_eq!(
196+
aabbs_2d(&nulls, &mut ctx)?.collect::<Vec<_>>(),
197+
vec![None, None]
198+
);
199+
Ok(())
200+
}
201+
142202
/// A zero-row column yields a zero-row result.
143203
#[test]
144204
fn empty_column_yields_nothing() -> VortexResult<()> {

vortex-geo/src/extension/linestring.rs

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ use arrow_array::ArrayRef as ArrowArrayRef;
1111
use arrow_schema::DataType;
1212
use arrow_schema::Field;
1313
use arrow_schema::extension::ExtensionType;
14-
use geo_traits::to_geo::ToGeoGeometry;
1514
use geo_types::Geometry;
1615
use geoarrow::array::GeoArrowArrayAccessor;
1716
use geoarrow::array::IntoArrow;
@@ -53,6 +52,7 @@ use super::coordinate::coordinate_storage_dtype;
5352
use super::geo_metadata_from_arrow;
5453
use super::geoarrow_metadata;
5554
use super::geoarrow_to_wkb;
55+
use super::geometry_row;
5656

5757
/// A line string: `geoarrow.linestring`, stored as `List<Struct<x, y[, z][, m]>>` (an ordered path
5858
/// of vertices).
@@ -111,20 +111,15 @@ fn linestring_type(geo_metadata: &GeoMetadata, dimension: Dimension) -> LineStri
111111
LineStringType::new(dimension.into(), geoarrow_metadata(geo_metadata))
112112
}
113113

114-
/// Decode `LineString` storage (`List<coordinate>`) to `geo_types` line strings, for the geo scalar
115-
/// functions. CRS does not affect planar geometry ops, so default metadata is used.
114+
/// Decode `LineString` storage (`List<coordinate>`) to `geo_types` line strings, one `Option` per
115+
/// row: a null row is `None`. CRS does not affect planar geometry ops, so default metadata is used.
116116
pub(crate) fn linestring_geometries(
117117
storage: &ArrayRef,
118118
ctx: &mut ExecutionCtx,
119-
) -> VortexResult<Vec<Geometry<f64>>> {
119+
) -> VortexResult<Vec<Option<Geometry<f64>>>> {
120120
linestring_array(storage, ctx)?
121121
.iter()
122-
.map(|geometry| -> VortexResult<Geometry<f64>> {
123-
Ok(geometry
124-
.ok_or_else(|| vortex_err!("geo: null geometry is not supported"))?
125-
.map_err(|e| vortex_err!("geo: geometry access failed: {e}"))?
126-
.to_geometry())
127-
})
122+
.map(geometry_row)
128123
.collect()
129124
}
130125

vortex-geo/src/extension/mod.rs

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use std::sync::Arc;
1616

1717
use ::wkb::reader::GeometryType;
1818
use arrow_array::BinaryArray;
19+
use geo_traits::to_geo::ToGeoGeometry;
1920
use geo_types::Geometry;
2021
use geoarrow::array::GenericWkbArray;
2122
use geoarrow::array::GeoArrowArray;
@@ -115,11 +116,25 @@ pub(crate) fn flatten_coordinates(
115116
node.execute::<StructArray>(ctx)
116117
}
117118

118-
/// Decode a native geometry column to `geo_types`. A non-geometry operand is an error.
119-
pub(crate) fn geometries(
119+
/// Decode one geoarrow row to `geo_types`: a null row is `None`; a geometry access failure is an
120+
/// error. The shared per-row step of the per-type decoders.
121+
pub(crate) fn geometry_row<T: ToGeoGeometry<f64>, E: Display>(
122+
row: Option<Result<T, E>>,
123+
) -> VortexResult<Option<Geometry<f64>>> {
124+
row.map(|geometry| {
125+
geometry
126+
.map(|g| g.to_geometry())
127+
.map_err(|e| vortex_err!("geo: geometry access failed: {e}"))
128+
})
129+
.transpose()
130+
}
131+
132+
/// Decode a native geometry column to `geo_types`, one `Option` per row: a null row is `None`.
133+
/// A non-geometry column is an error.
134+
pub(crate) fn geometries_opt(
120135
array: &ArrayRef,
121136
ctx: &mut ExecutionCtx,
122-
) -> VortexResult<Vec<Geometry<f64>>> {
137+
) -> VortexResult<Vec<Option<Geometry<f64>>>> {
123138
let Some(ext) = array.dtype().as_extension_opt() else {
124139
vortex_bail!(
125140
"geo: operand is not a geometry extension type, was {}",
@@ -150,6 +165,19 @@ pub(crate) fn geometries(
150165
}
151166
}
152167

168+
/// Decode a native geometry column to `geo_types`, erroring on any null row — the strict form of
169+
/// [`geometries_opt`] for callers that never decode nulls: the scalar functions reject nullable
170+
/// operands up front. A non-geometry column is an error.
171+
pub(crate) fn geometries(
172+
array: &ArrayRef,
173+
ctx: &mut ExecutionCtx,
174+
) -> VortexResult<Vec<Geometry<f64>>> {
175+
geometries_opt(array, ctx)?
176+
.into_iter()
177+
.map(|geometry| geometry.ok_or_else(|| vortex_err!("geo: null geometry is not supported")))
178+
.collect()
179+
}
180+
153181
/// Decode a constant operand scalar to one geo geometry, a constant of any
154182
/// supported geometry type is decoded exactly like a column.
155183
pub(crate) fn single_geometry(
@@ -296,6 +324,7 @@ pub(crate) fn geo_metadata_from_arrow(metadata: &Metadata) -> GeoMetadata {
296324
#[cfg(test)]
297325
mod tests {
298326
use prost::Message;
327+
use vortex_array::VortexSessionExecute;
299328
use vortex_array::dtype::DType;
300329
use vortex_error::VortexResult;
301330
use vortex_error::vortex_err;
@@ -305,8 +334,26 @@ mod tests {
305334
use super::MultiPoint;
306335
use super::Point;
307336
use super::Polygon;
337+
use super::geometries;
338+
use super::geometries_opt;
308339
use super::native_geometry_scalar_from_wkb;
309340
use crate::extension::GeoMetadata;
341+
use crate::test_harness::nullable_point_column;
342+
343+
/// A null row is `None` in the null-aware decode but an error in the strict one — the strict
344+
/// contract the scalar functions rely on must not shift.
345+
#[test]
346+
fn null_row_is_none_in_opt_but_errors_in_strict() -> VortexResult<()> {
347+
let session = vortex_array::array_session();
348+
let mut ctx = session.create_execution_ctx();
349+
350+
let points = nullable_point_column(vec![Some((1.0, 2.0)), None])?;
351+
let decoded = geometries_opt(&points, &mut ctx)?;
352+
assert!(decoded[0].is_some());
353+
assert!(decoded[1].is_none());
354+
assert!(geometries(&points, &mut ctx).is_err());
355+
Ok(())
356+
}
310357

311358
#[test]
312359
fn test_metadata() {

vortex-geo/src/extension/multilinestring.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ use arrow_array::ArrayRef as ArrowArrayRef;
1212
use arrow_schema::DataType;
1313
use arrow_schema::Field;
1414
use arrow_schema::extension::ExtensionType;
15-
use geo_traits::to_geo::ToGeoGeometry;
1615
use geo_types::Geometry;
1716
use geoarrow::array::GeoArrowArrayAccessor;
1817
use geoarrow::array::IntoArrow;
@@ -54,6 +53,7 @@ use super::coordinate::coordinate_storage_dtype;
5453
use super::geo_metadata_from_arrow;
5554
use super::geoarrow_metadata;
5655
use super::geoarrow_to_wkb;
56+
use super::geometry_row;
5757

5858
/// A multilinestring: `geoarrow.multilinestring`, stored as `List<List<Struct<x, y[, z][, m]>>>`
5959
/// (line strings of vertices).
@@ -116,19 +116,15 @@ fn multilinestring_type(geo_metadata: &GeoMetadata, dimension: Dimension) -> Mul
116116
MultiLineStringType::new(dimension.into(), geoarrow_metadata(geo_metadata))
117117
}
118118

119-
/// Decode storage to `geo_types` for the geo scalar functions (CRS is irrelevant to planar ops).
119+
/// Decode storage to `geo_types`, one `Option` per row: a null row is `None` (CRS is irrelevant
120+
/// to planar ops).
120121
pub(crate) fn multilinestring_geometries(
121122
storage: &ArrayRef,
122123
ctx: &mut ExecutionCtx,
123-
) -> VortexResult<Vec<Geometry<f64>>> {
124+
) -> VortexResult<Vec<Option<Geometry<f64>>>> {
124125
multilinestring_array(storage, ctx)?
125126
.iter()
126-
.map(|geometry| -> VortexResult<Geometry<f64>> {
127-
Ok(geometry
128-
.ok_or_else(|| vortex_err!("geo: null geometry is not supported"))?
129-
.map_err(|e| vortex_err!("geo: geometry access failed: {e}"))?
130-
.to_geometry())
131-
})
127+
.map(geometry_row)
132128
.collect()
133129
}
134130

vortex-geo/src/extension/multipoint.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ use arrow_array::ArrayRef as ArrowArrayRef;
1212
use arrow_schema::DataType;
1313
use arrow_schema::Field;
1414
use arrow_schema::extension::ExtensionType;
15-
use geo_traits::to_geo::ToGeoGeometry;
1615
use geo_types::Geometry;
1716
use geoarrow::array::GeoArrowArrayAccessor;
1817
use geoarrow::array::IntoArrow;
@@ -54,6 +53,7 @@ use super::coordinate::coordinate_storage_dtype;
5453
use super::geo_metadata_from_arrow;
5554
use super::geoarrow_metadata;
5655
use super::geoarrow_to_wkb;
56+
use super::geometry_row;
5757

5858
/// A multipoint: `geoarrow.multipoint`, stored as `List<Struct<x, y[, z][, m]>>` (a set of points).
5959
#[derive(Debug, Clone, Default, PartialEq, Eq, Hash)]
@@ -110,19 +110,15 @@ fn multipoint_type(geo_metadata: &GeoMetadata, dimension: Dimension) -> MultiPoi
110110
MultiPointType::new(dimension.into(), geoarrow_metadata(geo_metadata))
111111
}
112112

113-
/// Decode `MultiPoint` storage (`List<coordinate>`) to `geo_types`, for the geo scalar functions.
113+
/// Decode `MultiPoint` storage (`List<coordinate>`) to `geo_types`, one `Option` per row: a null
114+
/// row is `None`.
114115
pub(crate) fn multipoint_geometries(
115116
storage: &ArrayRef,
116117
ctx: &mut ExecutionCtx,
117-
) -> VortexResult<Vec<Geometry<f64>>> {
118+
) -> VortexResult<Vec<Option<Geometry<f64>>>> {
118119
multipoint_array(storage, ctx)?
119120
.iter()
120-
.map(|geometry| -> VortexResult<Geometry<f64>> {
121-
Ok(geometry
122-
.ok_or_else(|| vortex_err!("geo: null geometry is not supported"))?
123-
.map_err(|e| vortex_err!("geo: geometry access failed: {e}"))?
124-
.to_geometry())
125-
})
121+
.map(geometry_row)
126122
.collect()
127123
}
128124

vortex-geo/src/extension/multipolygon.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ use arrow_array::ArrayRef as ArrowArrayRef;
1111
use arrow_schema::DataType;
1212
use arrow_schema::Field;
1313
use arrow_schema::extension::ExtensionType;
14-
use geo_traits::to_geo::ToGeoGeometry;
1514
use geo_types::Geometry;
1615
use geoarrow::array::GeoArrowArrayAccessor;
1716
use geoarrow::array::IntoArrow;
@@ -53,6 +52,7 @@ use super::coordinate::coordinate_storage_dtype;
5352
use super::geo_metadata_from_arrow;
5453
use super::geoarrow_metadata;
5554
use super::geoarrow_to_wkb;
55+
use super::geometry_row;
5656

5757
/// A multipolygon (`geoarrow.multipolygon`); a single `Polygon` is a one-element multipolygon.
5858
#[derive(Debug, Clone, Default, PartialEq, Eq, Hash)]
@@ -117,19 +117,15 @@ fn multipolygon_type(geo_metadata: &GeoMetadata, dimension: Dimension) -> MultiP
117117
MultiPolygonType::new(dimension.into(), geoarrow_metadata(geo_metadata))
118118
}
119119

120-
/// Decode storage to `geo_types` for the geo scalar functions (CRS is irrelevant to planar ops).
120+
/// Decode storage to `geo_types`, one `Option` per row: a null row is `None` (CRS is irrelevant
121+
/// to planar ops).
121122
pub(crate) fn multipolygon_geometries(
122123
storage: &ArrayRef,
123124
ctx: &mut ExecutionCtx,
124-
) -> VortexResult<Vec<Geometry<f64>>> {
125+
) -> VortexResult<Vec<Option<Geometry<f64>>>> {
125126
multipolygon_array(storage, ctx)?
126127
.iter()
127-
.map(|geometry| -> VortexResult<Geometry<f64>> {
128-
Ok(geometry
129-
.ok_or_else(|| vortex_err!("geo: null geometry is not supported"))?
130-
.map_err(|e| vortex_err!("geo: geometry access failed: {e}"))?
131-
.to_geometry())
132-
})
128+
.map(geometry_row)
133129
.collect()
134130
}
135131

0 commit comments

Comments
 (0)