Skip to content

Commit 118c995

Browse files
committed
Document aggregate stat binding coverage
Signed-off-by: "Nicholas Gates" <nick@nickgates.com>
1 parent 0de2c09 commit 118c995

2 files changed

Lines changed: 101 additions & 5 deletions

File tree

vortex-array/src/stats/bind.rs

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@ pub trait StatBinder {
6464
aggregate_fn: &AggregateFnRef,
6565
stat_dtype: &DType,
6666
) -> VortexResult<Option<Expression>> {
67+
// These aggregate stats are derived from legacy count slots rather than
68+
// stored directly. Keep that storage mapping in binding so stats rewrite
69+
// rules can continue to ask for the semantic aggregate they need.
6770
if aggregate_fn.is::<AllNan>() {
6871
let Some(nan_count) = self.bind_legacy_stat(input, Stat::NaNCount)? else {
6972
return Ok(None);
@@ -163,3 +166,99 @@ fn bind_stat_fn(
163166
fn null_expr(dtype: DType) -> Expression {
164167
lit(Scalar::null(dtype.as_nullable()))
165168
}
169+
170+
#[cfg(test)]
171+
mod tests {
172+
use vortex_error::VortexResult;
173+
174+
use super::*;
175+
use crate::dtype::Nullability;
176+
use crate::dtype::PType;
177+
use crate::dtype::StructFields;
178+
use crate::expr::col;
179+
use crate::expr::get_item;
180+
use crate::expr::is_null;
181+
use crate::expr::root;
182+
use crate::stats::all_non_nan;
183+
184+
struct TestBinder {
185+
input_scope: DType,
186+
bound_scope: DType,
187+
bind_nan_count: bool,
188+
}
189+
190+
impl TestBinder {
191+
fn new(bind_nan_count: bool) -> Self {
192+
Self {
193+
input_scope: DType::Struct(
194+
StructFields::from_iter([(
195+
"f",
196+
DType::Primitive(PType::F32, Nullability::NonNullable),
197+
)]),
198+
Nullability::NonNullable,
199+
),
200+
bound_scope: DType::Struct(
201+
StructFields::from_iter([(
202+
"f_nan_count",
203+
DType::Primitive(PType::U64, Nullability::NonNullable),
204+
)]),
205+
Nullability::NonNullable,
206+
),
207+
bind_nan_count,
208+
}
209+
}
210+
}
211+
212+
impl StatBinder for TestBinder {
213+
fn scope(&self) -> &DType {
214+
&self.input_scope
215+
}
216+
217+
fn bound_scope(&self) -> DType {
218+
self.bound_scope.clone()
219+
}
220+
221+
fn bind_stat(
222+
&mut self,
223+
_input: &Expression,
224+
stat: Stat,
225+
_stat_dtype: &DType,
226+
) -> VortexResult<Option<Expression>> {
227+
if stat == Stat::NaNCount && self.bind_nan_count {
228+
Ok(Some(get_item("f_nan_count", root())))
229+
} else {
230+
Ok(None)
231+
}
232+
}
233+
}
234+
235+
#[test]
236+
fn all_non_nan_binds_to_nan_count_zero() -> VortexResult<()> {
237+
let mut binder = TestBinder::new(true);
238+
239+
let bound = bind_stats(all_non_nan(col("f")), &mut binder)?;
240+
241+
assert_eq!(bound, eq(col("f_nan_count"), lit(0u64)));
242+
Ok(())
243+
}
244+
245+
#[test]
246+
fn all_non_nan_lowers_to_null_when_nan_count_is_missing() -> VortexResult<()> {
247+
let mut binder = TestBinder::new(false);
248+
249+
let bound = bind_stats(all_non_nan(col("f")), &mut binder)?;
250+
251+
assert_eq!(bound, lit(Scalar::null(DType::Bool(Nullability::Nullable))));
252+
Ok(())
253+
}
254+
255+
#[test]
256+
fn unrelated_expressions_do_not_request_nan_count() -> VortexResult<()> {
257+
let mut binder = TestBinder::new(false);
258+
259+
let bound = bind_stats(is_null(col("f")), &mut binder)?;
260+
261+
assert_eq!(bound, is_null(col("f")));
262+
Ok(())
263+
}
264+
}

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -361,10 +361,7 @@ mod tests {
361361
.unwrap();
362362

363363
let mask = zone_map.prune(&all_null(root()), &SESSION).unwrap();
364-
assert_arrays_eq!(
365-
mask.into_array(),
366-
BoolArray::from_iter([false, true, true])
367-
);
364+
assert_arrays_eq!(mask.into_array(), BoolArray::from_iter([false, true, true]));
368365
}
369366

370367
#[test]
@@ -431,7 +428,7 @@ mod tests {
431428
}
432429

433430
#[test]
434-
fn float_min_max_stat_fn_requires_all_non_nan() {
431+
fn float_min_max_prunes_only_with_all_non_nan_proof() {
435432
let zone_map = ZoneMap::try_new(
436433
PType::F32.into(),
437434
StructArray::from_fields(&[

0 commit comments

Comments
 (0)