Skip to content

Commit 83b1332

Browse files
committed
Localize legacy stat aggregate binding
Signed-off-by: "Nicholas Gates" <nick@nickgates.com>
1 parent 9a737c9 commit 83b1332

3 files changed

Lines changed: 18 additions & 64 deletions

File tree

vortex-array/src/stats/bind.rs

Lines changed: 8 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ use crate::aggregate_fn::AggregateFnRef;
1919
use crate::dtype::DType;
2020
use crate::expr::Expression;
2121
use crate::expr::lit;
22-
use crate::expr::stats::Stat;
2322
use crate::expr::traversal::NodeExt;
2423
use crate::expr::traversal::Transformed;
2524
use crate::scalar::Scalar;
@@ -43,24 +42,10 @@ pub trait StatBinder {
4342
self.scope().clone()
4443
}
4544

46-
/// Bind `stat(input)` to a concrete expression.
47-
///
48-
/// Returning `Ok(None)` marks the stat as unavailable. [`bind_stats`] will
49-
/// then call [`Self::missing_stat`] with the dtype expected from the
50-
/// original `vortex.stat` expression.
51-
fn bind_stat(
52-
&self,
53-
input: &Expression,
54-
stat: Stat,
55-
stat_dtype: &DType,
56-
) -> VortexResult<Option<Expression>>;
57-
5845
/// Bind `aggregate_fn(input)` to a concrete expression.
5946
///
6047
/// Implementations should return `Ok(None)` when the requested aggregate
61-
/// statistic is unavailable in their backing representation. Binders that
62-
/// support only direct legacy [`Stat`] slots can delegate to
63-
/// [`bind_direct_aggregate_stat`].
48+
/// statistic is unavailable in their backing representation.
6449
fn bind_aggregate(
6550
&self,
6651
input: &Expression,
@@ -106,19 +91,6 @@ pub fn bind_stats<B: StatBinder + ?Sized>(
10691
lowered.optimize_recursive(&binder.bound_scope())
10792
}
10893

109-
/// Bind an aggregate function that has a direct legacy [`Stat`] slot.
110-
pub fn bind_direct_aggregate_stat<B: StatBinder + ?Sized>(
111-
binder: &B,
112-
input: &Expression,
113-
aggregate_fn: &AggregateFnRef,
114-
stat_dtype: &DType,
115-
) -> VortexResult<Option<Expression>> {
116-
let Some(stat) = Stat::from_aggregate_fn(aggregate_fn) else {
117-
return Ok(None);
118-
};
119-
binder.bind_stat(input, stat, stat_dtype)
120-
}
121-
12294
fn bind_stat_fn(
12395
expr: &Expression,
12496
scope: &DType,
@@ -151,6 +123,7 @@ mod tests {
151123
use crate::expr::is_null;
152124
use crate::expr::or;
153125
use crate::expr::root;
126+
use crate::expr::stats::Stat;
154127
use crate::stats::all_non_nan;
155128
use crate::stats::nan_count;
156129

@@ -191,27 +164,22 @@ mod tests {
191164
self.bound_scope.clone()
192165
}
193166

194-
fn bind_stat(
167+
fn bind_aggregate(
195168
&self,
196169
_input: &Expression,
197-
stat: Stat,
170+
aggregate_fn: &AggregateFnRef,
198171
_stat_dtype: &DType,
199172
) -> VortexResult<Option<Expression>> {
173+
let Some(stat) = Stat::from_aggregate_fn(aggregate_fn) else {
174+
return Ok(None);
175+
};
176+
200177
if stat == Stat::NaNCount && self.bind_nan_count {
201178
Ok(Some(get_item("f_nan_count", root())))
202179
} else {
203180
Ok(None)
204181
}
205182
}
206-
207-
fn bind_aggregate(
208-
&self,
209-
input: &Expression,
210-
aggregate_fn: &AggregateFnRef,
211-
stat_dtype: &DType,
212-
) -> VortexResult<Option<Expression>> {
213-
bind_direct_aggregate_stat(self, input, aggregate_fn, stat_dtype)
214-
}
215183
}
216184

217185
#[test]

vortex-file/src/pruning.rs

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ use vortex_array::scalar_fn::fns::get_item::GetItem;
2020
use vortex_array::scalar_fn::fns::literal::Literal;
2121
use vortex_array::scalar_fn::internal::row_count::substitute_row_count;
2222
use vortex_array::stats::bind::StatBinder;
23-
use vortex_array::stats::bind::bind_direct_aggregate_stat;
2423
use vortex_array::stats::bind::bind_stats;
2524
use vortex_error::VortexResult;
2625
use vortex_session::VortexSession;
@@ -80,26 +79,20 @@ impl StatBinder for FileStatsBinder<'_> {
8079
DType::Null
8180
}
8281

83-
fn bind_stat(
82+
fn bind_aggregate(
8483
&self,
8584
input: &Expression,
86-
stat: Stat,
85+
aggregate_fn: &AggregateFnRef,
8786
_stat_dtype: &DType,
8887
) -> VortexResult<Option<Expression>> {
88+
let Some(stat) = Stat::from_aggregate_fn(aggregate_fn) else {
89+
return Ok(None);
90+
};
8991
let Some(field_path) = direct_field_path(input) else {
9092
return Ok(None);
9193
};
9294
Ok(self.stat_ref(&field_path, stat))
9395
}
94-
95-
fn bind_aggregate(
96-
&self,
97-
input: &Expression,
98-
aggregate_fn: &AggregateFnRef,
99-
stat_dtype: &DType,
100-
) -> VortexResult<Option<Expression>> {
101-
bind_direct_aggregate_stat(self, input, aggregate_fn, stat_dtype)
102-
}
10396
}
10497

10598
impl FileStatsBinder<'_> {

vortex-layout/src/layouts/zoned/zone_map.rs

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ use vortex_array::expr::stats::Stat;
2222
use vortex_array::scalar_fn::internal::row_count::contains_row_count;
2323
use vortex_array::scalar_fn::internal::row_count::substitute_row_count;
2424
use vortex_array::stats::bind::StatBinder;
25-
use vortex_array::stats::bind::bind_direct_aggregate_stat;
2625
use vortex_array::stats::bind::bind_stats;
2726
use vortex_array::validity::Validity;
2827
use vortex_buffer::buffer;
@@ -139,12 +138,15 @@ impl StatBinder for ZoneMapStatsBinder<'_> {
139138
self.zone_map.array.dtype().clone()
140139
}
141140

142-
fn bind_stat(
141+
fn bind_aggregate(
143142
&self,
144143
input: &Expression,
145-
stat: Stat,
144+
aggregate_fn: &AggregateFnRef,
146145
_stat_dtype: &DType,
147146
) -> VortexResult<Option<Expression>> {
147+
let Some(stat) = Stat::from_aggregate_fn(aggregate_fn) else {
148+
return Ok(None);
149+
};
148150
if !is_root(input) {
149151
return Ok(None);
150152
}
@@ -158,15 +160,6 @@ impl StatBinder for ZoneMapStatsBinder<'_> {
158160
}
159161
Ok(Some(get_item(stat.name(), root())))
160162
}
161-
162-
fn bind_aggregate(
163-
&self,
164-
input: &Expression,
165-
aggregate_fn: &AggregateFnRef,
166-
stat_dtype: &DType,
167-
) -> VortexResult<Option<Expression>> {
168-
bind_direct_aggregate_stat(self, input, aggregate_fn, stat_dtype)
169-
}
170163
}
171164

172165
/// Build per-zone row counts for a zone map.

0 commit comments

Comments
 (0)