Skip to content

Commit 48d2bcc

Browse files
committed
Standardize scalar function array construction
Uses `ScalarFnFactoryExt` for tensor and geo scalar functions instead of maintaining per-function constructors. `L2Denorm` keeps its checked constructor because it validates the normalized-row invariant. Signed-off-by: "Connor Tsui" <connor.tsui20@gmail.com>
1 parent 08c336f commit 48d2bcc

11 files changed

Lines changed: 97 additions & 187 deletions

File tree

‎vortex-geo/benches/envelope.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ use vortex_array::Canonical;
2525
use vortex_array::ExecutionCtx;
2626
use vortex_array::IntoArray;
2727
use vortex_array::VortexSessionExecute;
28+
use vortex_array::arrays::scalar_fn::ScalarFnFactoryExt;
29+
use vortex_array::scalar_fn::EmptyOptions;
2830
use vortex_geo::scalar_fn::envelope::GeoEnvelope;
2931
use vortex_geo::test_harness::MultiPolygonRings;
3032
use vortex_geo::test_harness::geo_session;
@@ -58,9 +60,9 @@ fn coin(i: usize) -> bool {
5860

5961
/// Execute the envelope of `column` to completion.
6062
fn envelope(column: &ArrayRef, ctx: &mut ExecutionCtx) -> ArrayRef {
61-
GeoEnvelope::try_new_array(column.clone())
63+
GeoEnvelope
64+
.try_new_array(column.len(), EmptyOptions, [column.clone()])
6265
.unwrap()
63-
.into_array()
6466
.execute::<Canonical>(ctx)
6567
.unwrap()
6668
.into_array()

‎vortex-geo/src/scalar_fn/contains.rs‎

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
use geo::Contains;
77
use vortex_array::ArrayRef;
88
use vortex_array::ExecutionCtx;
9-
use vortex_array::arrays::ScalarFnArray;
109
use vortex_array::dtype::DType;
1110
use vortex_array::dtype::Nullability;
1211
use vortex_array::expr::Expression;
@@ -17,7 +16,6 @@ use vortex_array::scalar_fn::EmptyOptions;
1716
use vortex_array::scalar_fn::ExecutionArgs;
1817
use vortex_array::scalar_fn::ScalarFnId;
1918
use vortex_array::scalar_fn::ScalarFnVTable;
20-
use vortex_array::scalar_fn::TypedScalarFnInstance;
2119
use vortex_error::VortexResult;
2220
use vortex_session::VortexSession;
2321
use vortex_session::registry::CachedId;
@@ -31,17 +29,6 @@ use crate::scalar_fn::execute::execute_null_propagating;
3129
#[derive(Debug, Clone, Default, PartialEq, Eq, Hash)]
3230
pub struct GeoContains;
3331

34-
impl GeoContains {
35-
/// A lazy `ScalarFnArray` computing per-row whether operand `a` contains operand `b`;
36-
/// either may be constant. The output length is taken from `a`.
37-
pub fn try_new_array(a: ArrayRef, b: ArrayRef) -> VortexResult<ScalarFnArray> {
38-
ScalarFnArray::try_new(
39-
TypedScalarFnInstance::new(GeoContains, EmptyOptions).erased(),
40-
vec![a, b],
41-
)
42-
}
43-
}
44-
4532
impl ScalarFnVTable for GeoContains {
4633
type Options = EmptyOptions;
4734

@@ -115,6 +102,7 @@ mod tests {
115102
use vortex_array::VortexSessionExecute;
116103
use vortex_array::arrays::BoolArray;
117104
use vortex_array::arrays::ConstantArray;
105+
use vortex_array::arrays::scalar_fn::ScalarFnFactoryExt;
118106
use vortex_array::assert_arrays_eq;
119107
use vortex_array::dtype::DType;
120108
use vortex_array::dtype::Nullability;
@@ -164,7 +152,7 @@ mod tests {
164152
) -> VortexResult<()> {
165153
let session = vortex_array::array_session();
166154
let mut ctx = session.create_execution_ctx();
167-
let contains = GeoContains::try_new_array(a, b)?.into_array();
155+
let contains = GeoContains.try_new_array(a.len(), EmptyOptions, [a, b])?;
168156
assert_arrays_eq!(contains, BoolArray::from_iter(expected), &mut ctx);
169157
Ok(())
170158
}
@@ -271,7 +259,8 @@ mod tests {
271259

272260
let container = geometry_constant(&Geometry::Polygon(rect_polygon(0.0, 0.0, 4.0, 4.0)), 3)?;
273261
let points = nullable_point_column(vec![Some((2.0, 2.0)), None, Some((10.0, 10.0))])?;
274-
let contains = GeoContains::try_new_array(container, points)?.into_array();
262+
let contains =
263+
GeoContains.try_new_array(container.len(), EmptyOptions, [container, points])?;
275264

276265
let expected = BoolArray::new(
277266
BitBuffer::from_iter([true, false, false]),
@@ -291,7 +280,8 @@ mod tests {
291280
let point_dtype = point_column(vec![0.0], vec![0.0])?.dtype().as_nullable();
292281
let null_const = ConstantArray::new(Scalar::null(point_dtype), 2).into_array();
293282
let points = point_column(vec![2.0, 10.0], vec![2.0, 10.0])?;
294-
let contains = GeoContains::try_new_array(null_const, points)?.into_array();
283+
let contains =
284+
GeoContains.try_new_array(null_const.len(), EmptyOptions, [null_const, points])?;
295285

296286
let expected =
297287
BoolArray::new(BitBuffer::from_iter([false, false]), Validity::AllInvalid).into_array();
@@ -319,7 +309,8 @@ mod tests {
319309
None,
320310
Some((4.0, 4.0)),
321311
])?;
322-
let contains = GeoContains::try_new_array(container, contained)?.into_array();
312+
let contains =
313+
GeoContains.try_new_array(container.len(), EmptyOptions, [container, contained])?;
323314

324315
let expected = BoolArray::new(
325316
BitBuffer::from_iter([true, false, false, false]),
@@ -338,7 +329,8 @@ mod tests {
338329

339330
let container = geometry_constant(&Geometry::Polygon(rect_polygon(0.0, 0.0, 4.0, 4.0)), 2)?;
340331
let points = nullable_point_column(vec![None, None])?;
341-
let contains = GeoContains::try_new_array(container, points)?.into_array();
332+
let contains =
333+
GeoContains.try_new_array(container.len(), EmptyOptions, [container, points])?;
342334

343335
let expected =
344336
BoolArray::new(BitBuffer::from_iter([false, false]), Validity::AllInvalid).into_array();
@@ -355,7 +347,8 @@ mod tests {
355347

356348
let container = nullable_point_column(vec![Some((1.0, 1.0)), None])?;
357349
let contained = nullable_point_column(vec![None, Some((2.0, 2.0))])?;
358-
let contains = GeoContains::try_new_array(container, contained)?.into_array();
350+
let contains =
351+
GeoContains.try_new_array(container.len(), EmptyOptions, [container, contained])?;
359352

360353
let expected =
361354
BoolArray::new(BitBuffer::from_iter([false, false]), Validity::AllInvalid).into_array();

‎vortex-geo/src/scalar_fn/distance.rs‎

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ use geo::Distance;
77
use geo::Euclidean;
88
use vortex_array::ArrayRef;
99
use vortex_array::ExecutionCtx;
10-
use vortex_array::arrays::ScalarFnArray;
1110
use vortex_array::dtype::DType;
1211
use vortex_array::dtype::Nullability;
1312
use vortex_array::dtype::PType;
@@ -19,7 +18,6 @@ use vortex_array::scalar_fn::EmptyOptions;
1918
use vortex_array::scalar_fn::ExecutionArgs;
2019
use vortex_array::scalar_fn::ScalarFnId;
2120
use vortex_array::scalar_fn::ScalarFnVTable;
22-
use vortex_array::scalar_fn::TypedScalarFnInstance;
2321
use vortex_error::VortexResult;
2422
use vortex_session::VortexSession;
2523
use vortex_session::registry::CachedId;
@@ -32,17 +30,6 @@ use crate::scalar_fn::execute::execute_null_propagating;
3230
#[derive(Debug, Clone, Default, PartialEq, Eq, Hash)]
3331
pub struct GeoDistance;
3432

35-
impl GeoDistance {
36-
/// A lazy `ScalarFnArray` computing the per-row distance between operands `a` and `b`; either may
37-
/// be constant. The output length is taken from `a`.
38-
pub fn try_new_array(a: ArrayRef, b: ArrayRef) -> VortexResult<ScalarFnArray> {
39-
ScalarFnArray::try_new(
40-
TypedScalarFnInstance::new(GeoDistance, EmptyOptions).erased(),
41-
vec![a, b],
42-
)
43-
}
44-
}
45-
4633
impl ScalarFnVTable for GeoDistance {
4734
type Options = EmptyOptions;
4835

@@ -110,6 +97,7 @@ mod tests {
11097
use vortex_array::VortexSessionExecute;
11198
use vortex_array::arrays::ConstantArray;
11299
use vortex_array::arrays::PrimitiveArray;
100+
use vortex_array::arrays::scalar_fn::ScalarFnFactoryExt;
113101
use vortex_array::assert_arrays_eq;
114102
use vortex_array::dtype::DType;
115103
use vortex_array::dtype::Nullability;
@@ -153,7 +141,7 @@ mod tests {
153141

154142
let a = point_column(vec![0.0, 3.0, 0.0, 3.0], vec![0.0, 0.0, 4.0, 4.0])?;
155143
let b = point_constant(0.0, 0.0, 4, &mut ctx)?;
156-
let distance = GeoDistance::try_new_array(a, b)?.into_array();
144+
let distance = GeoDistance.try_new_array(a.len(), EmptyOptions, [a, b])?;
157145

158146
assert_eq!(distances(distance, &mut ctx)?, vec![0.0, 3.0, 4.0, 5.0]);
159147
Ok(())
@@ -167,7 +155,7 @@ mod tests {
167155

168156
let a = point_column(vec![0.0, 1.0], vec![0.0, 1.0])?;
169157
let b = point_column(vec![3.0, 1.0], vec![4.0, 1.0])?;
170-
let distance = GeoDistance::try_new_array(a, b)?.into_array();
158+
let distance = GeoDistance.try_new_array(a.len(), EmptyOptions, [a, b])?;
171159

172160
assert_eq!(distances(distance, &mut ctx)?, vec![5.0, 0.0]);
173161
Ok(())
@@ -181,7 +169,7 @@ mod tests {
181169

182170
let a = point_constant(0.0, 0.0, 4, &mut ctx)?;
183171
let b = point_column(vec![0.0, 3.0, 0.0, 3.0], vec![0.0, 0.0, 4.0, 4.0])?;
184-
let distance = GeoDistance::try_new_array(a, b)?.into_array();
172+
let distance = GeoDistance.try_new_array(a.len(), EmptyOptions, [a, b])?;
185173

186174
assert_eq!(distances(distance, &mut ctx)?, vec![0.0, 3.0, 4.0, 5.0]);
187175
Ok(())
@@ -195,7 +183,7 @@ mod tests {
195183

196184
let a = point_constant(0.0, 0.0, 3, &mut ctx)?;
197185
let b = point_constant(3.0, 4.0, 3, &mut ctx)?;
198-
let distance = GeoDistance::try_new_array(a, b)?.into_array();
186+
let distance = GeoDistance.try_new_array(a.len(), EmptyOptions, [a, b])?;
199187

200188
assert_eq!(distances(distance, &mut ctx)?, vec![5.0, 5.0, 5.0]);
201189
Ok(())
@@ -222,7 +210,7 @@ mod tests {
222210

223211
let a = nullable_point_column(vec![Some((0.0, 0.0)), None, Some((3.0, 4.0))])?;
224212
let b = point_constant(0.0, 0.0, 3, &mut ctx)?;
225-
let distance = GeoDistance::try_new_array(a, b)?.into_array();
213+
let distance = GeoDistance.try_new_array(a.len(), EmptyOptions, [a, b])?;
226214

227215
let expected = PrimitiveArray::new(
228216
vec![0.0f64, 0.0, 5.0],
@@ -241,7 +229,7 @@ mod tests {
241229

242230
let a = nullable_point_column(vec![Some((0.0, 0.0)), None, Some((0.0, 0.0))])?;
243231
let b = nullable_point_column(vec![Some((3.0, 4.0)), Some((1.0, 1.0)), None])?;
244-
let distance = GeoDistance::try_new_array(a, b)?.into_array();
232+
let distance = GeoDistance.try_new_array(a.len(), EmptyOptions, [a, b])?;
245233

246234
let expected = PrimitiveArray::new(
247235
vec![5.0f64, 0.0, 0.0],
@@ -261,7 +249,8 @@ mod tests {
261249
let point_dtype = point_column(vec![0.0], vec![0.0])?.dtype().as_nullable();
262250
let null_const = ConstantArray::new(Scalar::null(point_dtype), 3).into_array();
263251
let b = point_column(vec![0.0, 3.0, 0.0], vec![0.0, 0.0, 4.0])?;
264-
let distance = GeoDistance::try_new_array(null_const, b)?.into_array();
252+
let distance =
253+
GeoDistance.try_new_array(null_const.len(), EmptyOptions, [null_const, b])?;
265254

266255
let expected = PrimitiveArray::new(vec![0.0f64; 3], Validity::AllInvalid).into_array();
267256
assert_arrays_eq!(distance, expected, &mut ctx);
@@ -276,7 +265,7 @@ mod tests {
276265

277266
let a = nullable_point_column(vec![None, None])?;
278267
let b = point_constant(0.0, 0.0, 2, &mut ctx)?;
279-
let distance = GeoDistance::try_new_array(a, b)?.into_array();
268+
let distance = GeoDistance.try_new_array(a.len(), EmptyOptions, [a, b])?;
280269

281270
let expected = PrimitiveArray::new(vec![0.0f64; 2], Validity::AllInvalid).into_array();
282271
assert_arrays_eq!(distance, expected, &mut ctx);
@@ -292,7 +281,7 @@ mod tests {
292281

293282
let a = nullable_point_column(vec![Some((0.0, 0.0)), None])?;
294283
let b = nullable_point_column(vec![None, Some((1.0, 1.0))])?;
295-
let distance = GeoDistance::try_new_array(a, b)?.into_array();
284+
let distance = GeoDistance.try_new_array(a.len(), EmptyOptions, [a, b])?;
296285

297286
let expected = PrimitiveArray::new(vec![0.0f64; 2], Validity::AllInvalid).into_array();
298287
assert_arrays_eq!(distance, expected, &mut ctx);
@@ -309,8 +298,8 @@ mod tests {
309298

310299
let a = point_column(vec![], vec![])?;
311300
let b = point_column(vec![], vec![])?;
312-
let result = GeoDistance::try_new_array(a, b)?
313-
.into_array()
301+
let result = GeoDistance
302+
.try_new_array(a.len(), EmptyOptions, [a, b])?
314303
.execute::<Canonical>(&mut ctx)?
315304
.into_array();
316305

‎vortex-geo/src/scalar_fn/envelope.rs‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ use vortex_array::ArrayRef;
1010
use vortex_array::ExecutionCtx;
1111
use vortex_array::IntoArray;
1212
use vortex_array::arrays::ExtensionArray;
13-
use vortex_array::arrays::ScalarFnArray;
1413
use vortex_array::arrays::StructArray;
1514
use vortex_array::arrays::extension::ExtensionArrayExt;
1615
use vortex_array::arrays::struct_::StructArrayExt;
@@ -25,7 +24,6 @@ use vortex_array::scalar_fn::EmptyOptions;
2524
use vortex_array::scalar_fn::ExecutionArgs;
2625
use vortex_array::scalar_fn::ScalarFnId;
2726
use vortex_array::scalar_fn::ScalarFnVTable;
28-
use vortex_array::scalar_fn::TypedScalarFnInstance;
2927
use vortex_array::validity::Validity;
3028
use vortex_buffer::BitBuffer;
3129
use vortex_buffer::BufferMut;
@@ -53,17 +51,6 @@ use crate::extension::validate_geometry_operands;
5351
#[derive(Debug, Clone, Default, PartialEq, Eq, Hash)]
5452
pub struct GeoEnvelope;
5553

56-
impl GeoEnvelope {
57-
/// A lazy `ScalarFnArray` computing the per-row bounding box of geometry operand `a`, which may
58-
/// be constant. The output length is taken from `a`.
59-
pub fn try_new_array(a: ArrayRef) -> VortexResult<ScalarFnArray> {
60-
ScalarFnArray::try_new(
61-
TypedScalarFnInstance::new(GeoEnvelope, EmptyOptions).erased(),
62-
vec![a],
63-
)
64-
}
65-
}
66-
6754
/// The output dtype: a nullable native 2-D box ([`Rect`], `geoarrow.box`) column. Nullable
6855
/// because rows without a box — null or empty geometries — are null. Metadata is defaulted.
6956
fn output_box_dtype() -> VortexResult<ExtDType<Rect>> {
@@ -227,6 +214,7 @@ mod tests {
227214
use vortex_array::arrays::ExtensionArray;
228215
use vortex_array::arrays::PrimitiveArray;
229216
use vortex_array::arrays::StructArray;
217+
use vortex_array::arrays::scalar_fn::ScalarFnFactoryExt;
230218
use vortex_array::assert_arrays_eq;
231219
use vortex_array::dtype::DType;
232220
use vortex_array::dtype::Nullability;
@@ -255,7 +243,7 @@ mod tests {
255243

256244
/// Execute a `GeoEnvelope` over `array`, returning the lazy box column.
257245
fn boxes(array: ArrayRef) -> VortexResult<ArrayRef> {
258-
Ok(GeoEnvelope::try_new_array(array)?.into_array())
246+
GeoEnvelope.try_new_array(array.len(), EmptyOptions, [array])
259247
}
260248

261249
/// A point's box is degenerate: both corners are the point itself.

0 commit comments

Comments
 (0)