Skip to content

Commit f6c19be

Browse files
gatesngatesn
authored
Remove old operator API from vortex-expr (#5160)
Signed-off-by: Nicholas Gates <[email protected]>
1 parent 879a53b commit f6c19be

File tree

6 files changed

+2
-186
lines changed

6 files changed

+2
-186
lines changed

‎vortex-expr/src/exprs/between.rs‎

Lines changed: 2 additions & 142 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,13 @@
11
// SPDX-License-Identifier: Apache-2.0
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33

4-
use std::any::Any;
54
use std::fmt::Debug;
6-
use std::hash::{Hash, Hasher};
7-
use std::sync::Arc;
85

9-
use async_trait::async_trait;
10-
use futures::try_join;
11-
use itertools::Itertools;
126
use vortex_array::compute::{BetweenOptions, StrictComparison, between as between_compute};
13-
use vortex_array::operator::{
14-
BatchBindCtx, BatchExecution, BatchExecutionRef, BatchOperator, LengthBounds, Operator,
15-
OperatorEq, OperatorHash, OperatorId, OperatorRef,
16-
};
17-
use vortex_array::{Array, ArrayRef, Canonical, DeserializeMetadata, IntoArray, ProstMetadata};
7+
use vortex_array::{ArrayRef, DeserializeMetadata, ProstMetadata};
188
use vortex_dtype::DType;
199
use vortex_dtype::DType::Bool;
20-
use vortex_error::{VortexExpect, VortexResult, vortex_bail};
10+
use vortex_error::{VortexResult, vortex_bail};
2111
use vortex_proto::expr as pb;
2212

2313
use crate::display::{DisplayAs, DisplayFormat};
@@ -137,23 +127,6 @@ impl VTable for BetweenVTable {
137127
arr_dt.nullability() | lower_dt.nullability() | upper_dt.nullability(),
138128
))
139129
}
140-
141-
fn operator(expr: &Self::Expr, scope: &OperatorRef) -> VortexResult<Option<OperatorRef>> {
142-
let Some(arr) = expr.arr.operator(scope)? else {
143-
return Ok(None);
144-
};
145-
let Some(lower) = expr.lower.operator(scope)? else {
146-
return Ok(None);
147-
};
148-
let Some(upper) = expr.upper.operator(scope)? else {
149-
return Ok(None);
150-
};
151-
Ok(Some(Arc::new(BetweenOperator {
152-
children: [arr, lower, upper],
153-
dtype: expr.return_dtype(scope.dtype())?,
154-
options: expr.options.clone(),
155-
})))
156-
}
157130
}
158131

159132
impl BetweenExpr {
@@ -245,119 +218,6 @@ pub fn between(arr: ExprRef, lower: ExprRef, upper: ExprRef, options: BetweenOpt
245218
BetweenExpr::new(arr, lower, upper, options).into_expr()
246219
}
247220

248-
#[derive(Debug)]
249-
pub struct BetweenOperator {
250-
children: [OperatorRef; 3],
251-
dtype: DType,
252-
options: BetweenOptions,
253-
}
254-
255-
impl OperatorHash for BetweenOperator {
256-
fn operator_hash<H: Hasher>(&self, state: &mut H) {
257-
for child in &self.children {
258-
child.operator_hash(state);
259-
}
260-
self.dtype.hash(state);
261-
self.options.hash(state);
262-
}
263-
}
264-
265-
impl OperatorEq for BetweenOperator {
266-
fn operator_eq(&self, other: &Self) -> bool {
267-
self.children.len() == other.children.len()
268-
&& self
269-
.children
270-
.iter()
271-
.zip(other.children.iter())
272-
.all(|(a, b)| a.operator_eq(b))
273-
&& self.dtype == other.dtype
274-
&& self.options == other.options
275-
}
276-
}
277-
278-
impl Operator for BetweenOperator {
279-
fn id(&self) -> OperatorId {
280-
OperatorId::from("vortex.between")
281-
}
282-
283-
fn as_any(&self) -> &dyn Any {
284-
self
285-
}
286-
287-
fn dtype(&self) -> &DType {
288-
&self.dtype
289-
}
290-
291-
fn bounds(&self) -> LengthBounds {
292-
self.children[0].bounds()
293-
}
294-
295-
fn children(&self) -> &[OperatorRef] {
296-
&self.children
297-
}
298-
299-
fn with_children(self: Arc<Self>, children: Vec<OperatorRef>) -> VortexResult<OperatorRef> {
300-
let (arr, lower, upper) = children
301-
.into_iter()
302-
.tuples()
303-
.next()
304-
.vortex_expect("expected 3 children");
305-
306-
Ok(Arc::new(BetweenOperator {
307-
children: [arr, lower, upper],
308-
dtype: self.dtype.clone(),
309-
options: self.options.clone(),
310-
}))
311-
}
312-
313-
fn is_selection_target(&self, _child_idx: usize) -> Option<bool> {
314-
// All children are position preserving.
315-
Some(true)
316-
}
317-
}
318-
319-
impl BatchOperator for BetweenOperator {
320-
fn bind(&self, ctx: &mut dyn BatchBindCtx) -> VortexResult<BatchExecutionRef> {
321-
let arr = ctx.child(0)?;
322-
let lower = ctx.child(1)?;
323-
let upper = ctx.child(2)?;
324-
Ok(Box::new(BetweenExecution {
325-
arr,
326-
lower,
327-
upper,
328-
options: self.options.clone(),
329-
}))
330-
}
331-
}
332-
333-
struct BetweenExecution {
334-
arr: BatchExecutionRef,
335-
lower: BatchExecutionRef,
336-
upper: BatchExecutionRef,
337-
options: BetweenOptions,
338-
}
339-
340-
#[async_trait]
341-
impl BatchExecution for BetweenExecution {
342-
async fn execute(self: Box<Self>) -> VortexResult<Canonical> {
343-
let (arr, lower, upper) = try_join!(
344-
self.arr.execute(),
345-
self.lower.execute(),
346-
self.upper.execute()
347-
)?;
348-
let result = between_compute(
349-
arr.into_array().as_ref(),
350-
lower.into_array().as_ref(),
351-
upper.into_array().as_ref(),
352-
&self.options,
353-
)?;
354-
Ok(result.to_canonical())
355-
}
356-
}
357-
358-
// TODO(ngates): we need scalar variants for batch execution. Although really it should be
359-
// pipelined?
360-
361221
#[cfg(test)]
362222
mod tests {
363223
use vortex_array::compute::{BetweenOptions, StrictComparison};

‎vortex-expr/src/exprs/binary.rs‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,8 @@
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33

44
use std::hash::Hash;
5-
use std::sync::Arc;
65

76
use vortex_array::compute::{add, and_kleene, compare, div, mul, or_kleene, sub};
8-
use vortex_array::operator::OperatorRef;
9-
use vortex_array::operator::compare::CompareOperator;
107
use vortex_array::{ArrayRef, DeserializeMetadata, ProstMetadata, compute};
118
use vortex_dtype::DType;
129
use vortex_error::{VortexResult, vortex_bail};
@@ -116,19 +113,6 @@ impl VTable for BinaryVTable {
116113

117114
Ok(DType::Bool((lhs.is_nullable() || rhs.is_nullable()).into()))
118115
}
119-
120-
fn operator(expr: &BinaryExpr, scope: &OperatorRef) -> VortexResult<Option<OperatorRef>> {
121-
let Some(lhs) = expr.lhs.operator(scope)? else {
122-
return Ok(None);
123-
};
124-
let Some(rhs) = expr.rhs.operator(scope)? else {
125-
return Ok(None);
126-
};
127-
let Ok(op): VortexResult<compute::Operator> = expr.operator.try_into() else {
128-
return Ok(None);
129-
};
130-
Ok(Some(Arc::new(CompareOperator::try_new(lhs, rhs, op)?)))
131-
}
132116
}
133117

134118
impl BinaryExpr {

‎vortex-expr/src/exprs/literal.rs‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
// SPDX-License-Identifier: Apache-2.0
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33

4-
use std::sync::Arc;
5-
64
use vortex_array::arrays::ConstantArray;
7-
use vortex_array::operator::OperatorRef;
85
use vortex_array::{Array, ArrayRef, DeserializeMetadata, IntoArray, ProstMetadata};
96
use vortex_dtype::{DType, match_each_float_ptype};
107
use vortex_error::{VortexResult, vortex_bail, vortex_err};
@@ -78,14 +75,6 @@ impl VTable for LiteralVTable {
7875
fn return_dtype(expr: &Self::Expr, _scope: &DType) -> VortexResult<DType> {
7976
Ok(expr.value.dtype().clone())
8077
}
81-
82-
fn operator(expr: &Self::Expr, scope: &OperatorRef) -> VortexResult<Option<OperatorRef>> {
83-
let Some(len) = scope.bounds().maybe_len() else {
84-
// Cannot return unsized operator.
85-
return Ok(None);
86-
};
87-
Ok(Some(Arc::new(ConstantArray::new(expr.value.clone(), len))))
88-
}
8978
}
9079

9180
impl LiteralExpr {

‎vortex-expr/src/exprs/root.rs‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// SPDX-License-Identifier: Apache-2.0
22
// SPDX-FileCopyrightText: Copyright the Vortex contributors
33

4-
use vortex_array::operator::OperatorRef;
54
use vortex_array::stats::Stat;
65
use vortex_array::{ArrayRef, DeserializeMetadata, EmptyMetadata};
76
use vortex_dtype::{DType, FieldPath};
@@ -66,10 +65,6 @@ impl VTable for RootVTable {
6665
fn return_dtype(_expr: &Self::Expr, scope: &DType) -> VortexResult<DType> {
6766
Ok(scope.clone())
6867
}
69-
70-
fn operator(_expr: &Self::Expr, scope: &OperatorRef) -> VortexResult<Option<OperatorRef>> {
71-
Ok(Some(scope.clone()))
72-
}
7368
}
7469

7570
impl DisplayAs for RootExpr {

‎vortex-expr/src/lib.rs‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ pub use root::*;
5454
pub use scope::*;
5555
pub use scope_vars::*;
5656
pub use select::*;
57-
use vortex_array::operator::OperatorRef;
5857
use vortex_array::{Array, ArrayRef, SerializeMetadata};
5958
use vortex_dtype::{DType, FieldName, FieldPath};
6059
use vortex_error::{VortexExpect, VortexResult, VortexUnwrap, vortex_bail};
@@ -111,8 +110,6 @@ pub trait VortexExpr:
111110
/// Compute the type of the array returned by
112111
/// [`VortexExpr::evaluate`](./trait.VortexExpr.html#method.evaluate).
113112
fn return_dtype(&self, scope: &DType) -> VortexResult<DType>;
114-
115-
fn operator(&self, scope: &OperatorRef) -> VortexResult<Option<OperatorRef>>;
116113
}
117114

118115
dyn_hash::hash_trait_object!(VortexExpr);
@@ -277,10 +274,6 @@ impl<V: VTable> VortexExpr for ExprAdapter<V> {
277274
fn return_dtype(&self, scope: &DType) -> VortexResult<DType> {
278275
V::return_dtype(&self.0, scope)
279276
}
280-
281-
fn operator(&self, scope: &OperatorRef) -> VortexResult<Option<OperatorRef>> {
282-
V::operator(&self.0, scope)
283-
}
284277
}
285278

286279
impl<V: VTable> Debug for ExprAdapter<V> {

‎vortex-expr/src/vtable.rs‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ use std::fmt::Debug;
55
use std::hash::Hash;
66
use std::ops::Deref;
77

8-
use vortex_array::operator::OperatorRef;
98
use vortex_array::{ArrayRef, DeserializeMetadata, SerializeMetadata};
109
use vortex_dtype::DType;
1110
use vortex_error::VortexResult;
@@ -63,10 +62,6 @@ pub trait VTable: 'static + Sized + Send + Sync + Debug {
6362

6463
/// Compute the return [`DType`] of the expression if evaluated in the given scope.
6564
fn return_dtype(expr: &Self::Expr, scope: &DType) -> VortexResult<DType>;
66-
67-
fn operator(_expr: &Self::Expr, _scope: &OperatorRef) -> VortexResult<Option<OperatorRef>> {
68-
Ok(None)
69-
}
7065
}
7166

7267
#[macro_export]

0 commit comments

Comments
 (0)