Skip to content

Commit ffbccee

Browse files
committed
Remove legacy stat expression hooks
Signed-off-by: Nicholas Gates <nick@nickgates.com>
1 parent 4293838 commit ffbccee

10 files changed

Lines changed: 0 additions & 205 deletions

File tree

vortex-array/src/expr/expression.rs

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,7 @@ use vortex_error::vortex_ensure;
1515
use vortex_session::VortexSession;
1616

1717
use crate::dtype::DType;
18-
use crate::expr::StatsCatalog;
1918
use crate::expr::display::DisplayTreeExpr;
20-
use crate::expr::stats::Stat;
2119
use crate::scalar_fn::ScalarFnRef;
2220
use crate::scalar_fn::fns::root::Root;
2321

@@ -142,28 +140,6 @@ impl Expression {
142140
crate::stats::rewrite::StatsRewriteCtx::new(session, scope).satisfy(self)
143141
}
144142

145-
/// Returns an expression representing the zoned statistic for the given stat, if available.
146-
///
147-
/// The [`StatsCatalog`] returns expressions that can be evaluated using the zone map as a
148-
/// scope. Expressions can implement this function to propagate such statistics through the
149-
/// expression tree. For example, the `a + 10` expression could propagate `min: min(a) + 10`.
150-
///
151-
/// NOTE(gatesn): we currently cannot represent statistics over nested fields. Please file an
152-
/// issue to discuss a solution to this.
153-
pub fn stat_expression(&self, stat: Stat, catalog: &dyn StatsCatalog) -> Option<Expression> {
154-
self.scalar_fn().stat_expression(self, stat, catalog)
155-
}
156-
157-
/// Returns an expression representing the zoned maximum statistic, if available.
158-
pub fn stat_min(&self, catalog: &dyn StatsCatalog) -> Option<Expression> {
159-
self.stat_expression(Stat::Min, catalog)
160-
}
161-
162-
/// Returns an expression representing the zoned maximum statistic, if available.
163-
pub fn stat_max(&self, catalog: &dyn StatsCatalog) -> Option<Expression> {
164-
self.stat_expression(Stat::Max, catalog)
165-
}
166-
167143
/// Format the expression as a compact string.
168144
///
169145
/// Since this is a recursive formatter, it is exposed on the public Expression type.

vortex-array/src/expr/mod.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ pub mod traversal;
4242
pub use analysis::*;
4343
pub use expression::*;
4444
pub use exprs::*;
45-
pub use pruning::StatsCatalog;
4645

4746
pub trait VortexExprExt {
4847
/// Accumulate all field references from this expression and its children in a set

vortex-array/src/expr/pruning/mod.rs

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,3 @@ pub use pruning_expr::RequiredStats;
88
pub use pruning_expr::checked_pruning_expr;
99
pub use pruning_expr::field_path_stat_field_name;
1010
pub use relation::Relation;
11-
12-
use crate::dtype::FieldPath;
13-
use crate::expr::Expression;
14-
use crate::expr::stats::Stat;
15-
16-
/// A catalog of available stats that are associated with field paths.
17-
pub trait StatsCatalog {
18-
/// Given a field path and statistic, return an expression that when evaluated over the catalog
19-
/// will return that stat for the referenced field.
20-
///
21-
/// This is likely to be a column expression, or a literal.
22-
///
23-
/// Returns `None` if the stat is not available for the field path.
24-
fn stats_ref(&self, _field_path: &FieldPath, _stat: Stat) -> Option<Expression> {
25-
None
26-
}
27-
}

vortex-array/src/scalar_fn/erased.rs

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ use crate::ArrayRef;
2020
use crate::ExecutionCtx;
2121
use crate::dtype::DType;
2222
use crate::expr::Expression;
23-
use crate::expr::StatsCatalog;
24-
use crate::expr::stats::Stat;
2523
use crate::scalar_fn::EmptyOptions;
2624
use crate::scalar_fn::ExecutionArgs;
2725
use crate::scalar_fn::ReduceCtx;
@@ -180,16 +178,6 @@ impl ScalarFnRef {
180178
pub(crate) fn simplify_untyped(&self, expr: &Expression) -> VortexResult<Option<Expression>> {
181179
self.0.simplify_untyped(expr)
182180
}
183-
184-
/// Compute stat expression.
185-
pub(crate) fn stat_expression(
186-
&self,
187-
expr: &Expression,
188-
stat: Stat,
189-
catalog: &dyn StatsCatalog,
190-
) -> Option<Expression> {
191-
self.0.stat_expression(expr, stat, catalog)
192-
}
193181
}
194182

195183
impl Debug for ScalarFnRef {

vortex-array/src/scalar_fn/fns/cast/mod.rs

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,8 @@ use crate::arrays::VarBinView;
3232
use crate::arrays::struct_::compute::cast::struct_cast;
3333
use crate::builtins::ArrayBuiltins;
3434
use crate::dtype::DType;
35-
use crate::expr::StatsCatalog;
36-
use crate::expr::cast;
3735
use crate::expr::expression::Expression;
3836
use crate::expr::lit;
39-
use crate::expr::stats::Stat;
4037
use crate::scalar_fn::Arity;
4138
use crate::scalar_fn::ChildName;
4239
use crate::scalar_fn::ExecutionArgs;
@@ -152,39 +149,6 @@ impl ScalarFnVTable for Cast {
152149
Ok(None)
153150
}
154151

155-
fn stat_expression(
156-
&self,
157-
dtype: &DType,
158-
expr: &Expression,
159-
stat: Stat,
160-
catalog: &dyn StatsCatalog,
161-
) -> Option<Expression> {
162-
match stat {
163-
Stat::IsConstant
164-
| Stat::IsSorted
165-
| Stat::IsStrictSorted
166-
| Stat::NaNCount
167-
| Stat::Sum
168-
| Stat::UncompressedSizeInBytes => expr.child(0).stat_expression(stat, catalog),
169-
Stat::Max | Stat::Min => {
170-
// We cast min/max to the new type
171-
expr.child(0)
172-
.stat_expression(stat, catalog)
173-
.map(|x| cast(x, dtype.clone()))
174-
}
175-
Stat::NullCount => {
176-
// if !expr.data().is_nullable() {
177-
// NOTE(ngates): we should decide on the semantics here. In theory, the null
178-
// count of something cast to non-nullable will be zero. But if we return
179-
// that we know this to be zero, then a pruning predicate may eliminate data
180-
// that would otherwise have caused the cast to error.
181-
// return Some(lit(0u64));
182-
// }
183-
None
184-
}
185-
}
186-
}
187-
188152
fn validity(&self, dtype: &DType, expression: &Expression) -> VortexResult<Option<Expression>> {
189153
Ok(Some(if dtype.is_nullable() {
190154
expression.child(0).validity()?

vortex-array/src/scalar_fn/fns/get_item.rs

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,9 @@ use crate::builtins::ArrayBuiltins;
1818
use crate::builtins::ExprBuiltins;
1919
use crate::dtype::DType;
2020
use crate::dtype::FieldName;
21-
use crate::dtype::FieldPath;
2221
use crate::dtype::Nullability;
2322
use crate::expr::Expression;
24-
use crate::expr::StatsCatalog;
2523
use crate::expr::lit;
26-
use crate::expr::stats::Stat;
2724
use crate::scalar_fn::Arity;
2825
use crate::scalar_fn::ChildName;
2926
use crate::scalar_fn::EmptyOptions;
@@ -188,24 +185,6 @@ impl ScalarFnVTable for GetItem {
188185
Ok(None)
189186
}
190187

191-
fn stat_expression(
192-
&self,
193-
field_name: &FieldName,
194-
_expr: &Expression,
195-
stat: Stat,
196-
catalog: &dyn StatsCatalog,
197-
) -> Option<Expression> {
198-
// TODO(ngates): I think we can do better here and support stats over nested fields.
199-
// It would be nice if delegating to our child would return a struct of statistics
200-
// matching the nested DType such that we can write:
201-
// `get_item(expr.child(0).stat_expression(...), expr.data().field_name())`
202-
203-
// TODO(ngates): this is a bug whereby we may return stats for a nested field of the same
204-
// name as a field in the root struct. This should be resolved with upcoming change to
205-
// falsify expressions, but for now I'm preserving the existing buggy behavior.
206-
catalog.stats_ref(&FieldPath::from_name(field_name.clone()), stat)
207-
}
208-
209188
// This will apply struct nullability field. We could add a dtype??
210189
fn is_null_sensitive(&self, _field_name: &FieldName) -> bool {
211190
true

vortex-array/src/scalar_fn/fns/literal.rs

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,6 @@ use crate::IntoArray;
1616
use crate::arrays::ConstantArray;
1717
use crate::dtype::DType;
1818
use crate::expr::Expression;
19-
use crate::expr::StatsCatalog;
20-
use crate::expr::stats::Stat;
21-
use crate::match_each_float_ptype;
2219
use crate::scalar::Scalar;
2320
use crate::scalar_fn::Arity;
2421
use crate::scalar_fn::ChildName;
@@ -96,50 +93,6 @@ impl ScalarFnVTable for Literal {
9693
Ok(ConstantArray::new(scalar.clone(), args.row_count()).into_array())
9794
}
9895

99-
fn stat_expression(
100-
&self,
101-
scalar: &Scalar,
102-
_expr: &Expression,
103-
stat: Stat,
104-
_catalog: &dyn StatsCatalog,
105-
) -> Option<Expression> {
106-
// NOTE(ngates): we return incorrect `1` values for counts here since we don't have
107-
// row-count information. We could resolve this in the future by introducing a `count()`
108-
// expression that evaluates to the row count of the provided scope. But since this is
109-
// only currently used for pruning, it doesn't change the outcome.
110-
111-
match stat {
112-
Stat::Min | Stat::Max => Some(lit(scalar.clone())),
113-
Stat::IsConstant => Some(lit(true)),
114-
Stat::NaNCount => {
115-
// The NaNCount for a non-float literal is not defined.
116-
// For floating point types, the NaNCount is 1 for lit(NaN), and 0 otherwise.
117-
let value = scalar.as_primitive_opt()?;
118-
if !value.ptype().is_float() {
119-
return None;
120-
}
121-
122-
match_each_float_ptype!(value.ptype(), |T| {
123-
if value.typed_value::<T>().is_some_and(|v| v.is_nan()) {
124-
Some(lit(1u64))
125-
} else {
126-
Some(lit(0u64))
127-
}
128-
})
129-
}
130-
Stat::NullCount => {
131-
if scalar.is_null() {
132-
Some(lit(1u64))
133-
} else {
134-
Some(lit(0u64))
135-
}
136-
}
137-
Stat::IsSorted | Stat::IsStrictSorted | Stat::Sum | Stat::UncompressedSizeInBytes => {
138-
None
139-
}
140-
}
141-
}
142-
14396
fn validity(
14497
&self,
14598
scalar: &Scalar,

vortex-array/src/scalar_fn/fns/root.rs

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,7 @@ use vortex_session::registry::CachedId;
1111
use crate::ArrayRef;
1212
use crate::ExecutionCtx;
1313
use crate::dtype::DType;
14-
use crate::dtype::FieldPath;
15-
use crate::expr::StatsCatalog;
1614
use crate::expr::expression::Expression;
17-
use crate::expr::stats::Stat;
1815
use crate::scalar_fn::Arity;
1916
use crate::scalar_fn::ChildName;
2017
use crate::scalar_fn::EmptyOptions;
@@ -80,16 +77,6 @@ impl ScalarFnVTable for Root {
8077
vortex_bail!("Root expression is not executable")
8178
}
8279

83-
fn stat_expression(
84-
&self,
85-
_options: &Self::Options,
86-
_expr: &Expression,
87-
stat: Stat,
88-
catalog: &dyn StatsCatalog,
89-
) -> Option<Expression> {
90-
catalog.stats_ref(&FieldPath::root(), stat)
91-
}
92-
9380
fn is_null_sensitive(&self, _options: &Self::Options) -> bool {
9481
false
9582
}

vortex-array/src/scalar_fn/typed.rs

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ use crate::ArrayRef;
2222
use crate::ExecutionCtx;
2323
use crate::dtype::DType;
2424
use crate::expr::Expression;
25-
use crate::expr::StatsCatalog;
26-
use crate::expr::stats::Stat;
2725
use crate::scalar_fn::Arity;
2826
use crate::scalar_fn::ChildName;
2927
use crate::scalar_fn::ExecutionArgs;
@@ -101,12 +99,6 @@ pub(super) trait DynScalarFn: 'static + Send + Sync + super::sealed::Sealed {
10199
) -> VortexResult<Option<Expression>>;
102100
fn simplify_untyped(&self, expression: &Expression) -> VortexResult<Option<Expression>>;
103101
fn validity(&self, expression: &Expression) -> VortexResult<Option<Expression>>;
104-
fn stat_expression(
105-
&self,
106-
expression: &Expression,
107-
stat: Stat,
108-
catalog: &dyn StatsCatalog,
109-
) -> Option<Expression>;
110102

111103
// Options operations — self-contained
112104
fn options_serialize(&self) -> VortexResult<Option<Vec<u8>>>;
@@ -218,15 +210,6 @@ impl<V: ScalarFnVTable> DynScalarFn for TypedScalarFnInstance<V> {
218210
V::validity(&self.vtable, &self.options, expression)
219211
}
220212

221-
fn stat_expression(
222-
&self,
223-
expression: &Expression,
224-
stat: Stat,
225-
catalog: &dyn StatsCatalog,
226-
) -> Option<Expression> {
227-
V::stat_expression(&self.vtable, &self.options, expression, stat, catalog)
228-
}
229-
230213
fn options_serialize(&self) -> VortexResult<Option<Vec<u8>>> {
231214
V::serialize(&self.vtable, &self.options)
232215
}

vortex-array/src/scalar_fn/vtable.rs

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ use crate::ArrayRef;
2020
use crate::ExecutionCtx;
2121
use crate::dtype::DType;
2222
use crate::expr::Expression;
23-
use crate::expr::StatsCatalog;
24-
use crate::expr::stats::Stat;
2523
use crate::expr::traversal::Node;
2624
use crate::scalar_fn::ScalarFnId;
2725
use crate::scalar_fn::ScalarFnRef;
@@ -179,21 +177,6 @@ pub trait ScalarFnVTable: 'static + Sized + Clone + Send + Sync {
179177
Ok(None)
180178
}
181179

182-
/// See [`Expression::stat_expression`].
183-
fn stat_expression(
184-
&self,
185-
options: &Self::Options,
186-
expr: &Expression,
187-
stat: Stat,
188-
catalog: &dyn StatsCatalog,
189-
) -> Option<Expression> {
190-
_ = options;
191-
_ = expr;
192-
_ = stat;
193-
_ = catalog;
194-
None
195-
}
196-
197180
/// Returns an expression that evaluates to the validity of the result of this expression.
198181
///
199182
/// If a validity expression cannot be constructed, returns `None` and the expression will

0 commit comments

Comments
 (0)