Address stats binding review comments

Signed-off-by: "Nicholas Gates" <nick@nickgates.com>

Author: gatesn

.github/workflows/ci.yml

@@ -368,10 +368,6 @@ jobs:
         with:
           sccache: s3
       - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10  # v6
-      - uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654  # v5
-        with:
-          distribution: "corretto"
-          java-version: "17"
       - uses: ./.github/actions/setup-prebuild
       - run: ./gradlew javadoc
         working-directory: ./java

docs/developer-guide/index.md

@@ -24,6 +24,7 @@ internals/session
 internals/async-runtime
 internals/vtables
 internals/execution
+internals/stats-pruning
 internals/io
 internals/serialization
 internals/cuda
@@ -38,4 +39,4 @@ caption: Integrations
 integrations/datafusion
 integrations/duckdb
 integrations/spark
-```
+```

docs/developer-guide/internals/stats-pruning.md

@@ -0,0 +1,40 @@
+# Stats Pruning
+
+Vortex uses statistics to prove when a filter cannot match a row group, zone, or
+file. The proof expression returns `true` when the input can be skipped. It
+returns `false` or `null` when pruning is not proven.
+
+The pruning pipeline has two phases:
+
+1. `Expression::falsify(scope, session)` asks the session's
+   `StatsRewriteRule`s to rewrite a filter into an abstract proof expression.
+   Rules describe semantics in terms of `vortex.stat(input, aggregate_fn)`
+   placeholders, so the rewrite does not depend on a particular layout.
+2. `bind_stats` lowers those abstract stat placeholders with a `StatBinder`.
+   The binder maps stats to the representation used by the caller, such as
+   zone-map table fields, file-level stat literals, or typed null literals for
+   missing stats.
+
+Missing stats lower to typed null literals. This preserves the three-valued
+logic used by pruning: only a non-null `true` value proves that the scope can be
+skipped. A missing stat therefore cannot accidentally prune data.
+
+## Binding Targets
+
+Zone maps bind stats to fields in their per-zone stats table. The lowered
+expression is evaluated against that table and produces a mask where `true`
+means the zone can be skipped.
+
+File-level stats bind stats to literal values from the file footer. The lowered
+expression is reduced and evaluated once for the full file. If it evaluates to
+`true`, the file stats reader can return an all-false pruning mask without
+reading child layouts.
+
+Scan planning uses `checked_pruning_expr` to lower a falsified expression against
+the available stats table schema. It returns the stats-table expression and the
+set of stat fields still required after expression reduction. If all required
+stats are missing, only a constant `true` proof is useful; all other results are
+treated as no pruning expression.
+
+For the layout model around these pruning points, see
+[Layouts](../../concepts/layouts.md) and [Scanning](../../concepts/scanning.md).

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

@@ -14,6 +14,8 @@ use crate::dtype::Field;
 use crate::dtype::FieldName;
 use crate::dtype::FieldPath;
 use crate::dtype::FieldPathSet;
+use crate::dtype::Nullability;
+use crate::dtype::StructFields;
 use crate::expr::Expression;
 use crate::expr::analysis::referenced_field_paths;
 use crate::expr::get_item;
@@ -70,11 +72,13 @@ pub fn checked_pruning_expr(
         scope,
         available_stats,
         required_stats: Relation::new(),
+        bound_stats: Vec::new(),
     };
-    let Some(lowered) = bind_stats(predicate, &mut binder)? else {
-        return Ok(None);
-    };
+    let lowered = bind_stats(predicate, &mut binder)?;
     let required_stats = filter_required_stats(&lowered, binder.required_stats);
+    // If no stats-table fields remain, only a constant `true` proof can prune.
+    // `false`, `null`, and non-constant expressions cannot justify building a
+    // stats-table pruning expression.
     if required_stats.map().is_empty() && !matches!(bool_literal(&lowered), Some(Some(true))) {
         return Ok(None);
     }
@@ -86,18 +90,26 @@ struct RequiredStatsBinder<'a> {
     scope: &'a DType,
     available_stats: &'a FieldPathSet,
     required_stats: RequiredStats,
+    bound_stats: Vec<(FieldName, DType)>,
 }
 
 impl StatBinder for RequiredStatsBinder<'_> {
     fn scope(&self) -> &DType {
         self.scope
     }
 
+    fn bound_scope(&self) -> DType {
+        DType::Struct(
+            StructFields::from_iter(self.bound_stats.iter().cloned()),
+            Nullability::NonNullable,
+        )
+    }
+
     fn bind_stat(
         &mut self,
         input: &Expression,
         stat: Stat,
-        _stat_dtype: &DType,
+        stat_dtype: &DType,
     ) -> VortexResult<Option<Expression>> {
         let field_path = match direct_stat_field_path(input) {
             Some(field_path) => field_path,
@@ -114,11 +126,18 @@ impl StatBinder for RequiredStatsBinder<'_> {
             return Ok(None);
         }
 
-        self.required_stats.insert(field_path.clone(), stat);
-        Ok(Some(get_item(
-            field_path_stat_field_name(&field_path, stat),
-            root(),
-        )))
+        let stat_field_name = field_path_stat_field_name(&field_path, stat);
+        if self
+            .bound_stats
+            .iter()
+            .all(|(field_name, _)| field_name != stat_field_name)
+        {
+            self.bound_stats
+                .push((stat_field_name.clone(), stat_dtype.clone()));
+        }
+
+        self.required_stats.insert(field_path, stat);
+        Ok(Some(get_item(stat_field_name, root())))
     }
 }
 

vortex-array/src/stats/bind.rs

@@ -2,6 +2,11 @@
 // SPDX-FileCopyrightText: Copyright the Vortex contributors
 
 //! Bind abstract `vortex.stat` expressions to a concrete stats representation.
+//!
+//! Stats rewrite rules describe pruning in terms of `vortex.stat(input, aggregate_fn)` placeholders
+//! so the rewrite is independent of where statistics are stored. Binding is the later pass that
+//! replaces those placeholders with the representation used by a caller: zone-map field references,
+//! file-level stat literals, or typed nulls for missing stats.
 
 use vortex_error::VortexResult;
 
@@ -20,6 +25,14 @@ pub trait StatBinder {
     /// The dtype scope used to type-check expressions before stats are bound.
     fn scope(&self) -> &DType;
 
+    /// The dtype scope used after stats have been bound.
+    ///
+    /// Binders that rewrite stats to a different root expression, such as a
+    /// stats-table row, should return that post-binding root dtype.
+    fn bound_scope(&self) -> DType {
+        self.scope().clone()
+    }
+
     /// Bind `stat(input)` to a concrete expression.
     ///
     /// Returning `Ok(None)` marks the stat as unavailable. [`bind_stats`] will
@@ -52,10 +65,9 @@ pub trait StatBinder {
     /// Expression to use when a stat is unavailable.
     ///
     /// The default is a nullable null literal, which preserves three-valued
-    /// pruning semantics for stats-table execution. Catalog-like binders can
-    /// return `Ok(None)` to reject expressions that require unavailable stats.
-    fn missing_stat(&mut self, dtype: DType) -> VortexResult<Option<Expression>> {
-        Ok(Some(null_expr(dtype)))
+    /// pruning semantics for stats-table execution.
+    fn missing_stat(&mut self, dtype: DType) -> VortexResult<Expression> {
+        Ok(null_expr(dtype))
     }
 }
 
@@ -64,10 +76,7 @@ pub trait StatBinder {
 /// The predicate is usually the output of a stats rewrite rule. Rewrite rules
 /// are responsible for expressing stat semantics; binding maps aggregate-backed
 /// stat requests to the concrete stats representation supported by the binder.
-pub fn bind_stats(
-    predicate: Expression,
-    binder: &mut impl StatBinder,
-) -> VortexResult<Option<Expression>> {
+pub fn bind_stats(predicate: Expression, binder: &mut impl StatBinder) -> VortexResult<Expression> {
     let scope = binder.scope().clone();
     let lowered = predicate
         .transform_down(|expr| {
@@ -79,18 +88,13 @@ pub fn bind_stats(
                 Some(bound) => Ok(Transformed::yes(bound)),
                 None => {
                     let dtype = expr.return_dtype(&scope)?;
-                    match binder.missing_stat(dtype.clone())? {
-                        Some(missing) => Ok(Transformed::yes(missing)),
-                        None => Ok(Transformed::yes(null_expr(dtype))),
-                    }
+                    Ok(Transformed::yes(binder.missing_stat(dtype)?))
                 }
             }
         })?
         .into_inner();
 
-    #[expect(deprecated)]
-    let lowered = lowered.simplify_untyped()?;
-    Ok(Some(lowered))
+    lowered.optimize_recursive(&binder.bound_scope())
 }
 
 fn bind_stat_fn(
@@ -100,6 +104,7 @@ fn bind_stat_fn(
 ) -> VortexResult<Option<Expression>> {
     let options = expr.as_::<StatFn>();
     let aggregate_fn = options.aggregate_fn();
+    // `StatFn` has exactly one child: the expression the aggregate statistic is computed over.
     let input = expr.child(0);
 
     let stat_dtype = expr.return_dtype(scope)?;

vortex-array/src/stats/rewrite/builtins.rs

@@ -814,6 +814,33 @@ mod tests {
             ))
         );
 
+        let expr = like(col("s"), lit(r"\%%"));
+        assert_eq!(
+            falsify(&expr)?,
+            Some(or(
+                gt_eq(stat(col("s"), Stat::Min), lit("&")),
+                lt(stat(col("s"), Stat::Max), lit("%")),
+            ))
+        );
+
+        let expr = like(col("s"), lit("pref%ix%"));
+        assert_eq!(
+            falsify(&expr)?,
+            Some(or(
+                gt_eq(stat(col("s"), Stat::Min), lit("preg")),
+                lt(stat(col("s"), Stat::Max), lit("pref")),
+            ))
+        );
+
+        let expr = like(col("s"), lit("pref_ix_"));
+        assert_eq!(
+            falsify(&expr)?,
+            Some(or(
+                gt_eq(stat(col("s"), Stat::Min), lit("preg")),
+                lt(stat(col("s"), Stat::Max), lit("pref")),
+            ))
+        );
+
         let expr = like(col("s"), lit("exact"));
         assert_eq!(
             falsify(&expr)?,

vortex-file/src/v2/file_stats_reader.rs

@@ -31,7 +31,6 @@ use vortex_array::scalar_fn::internal::row_count::substitute_row_count;
 use vortex_array::stats::bind::StatBinder;
 use vortex_array::stats::bind::bind_stats;
 use vortex_error::VortexResult;
-use vortex_error::vortex_bail;
 use vortex_layout::ArrayFuture;
 use vortex_layout::LayoutReader;
 use vortex_layout::LayoutReaderRef;
@@ -121,10 +120,7 @@ impl FileStatsLayoutReader {
 
     fn lower_stats(&self, predicate: Expression) -> VortexResult<Expression> {
         let mut binder = FileStatsBinder { reader: self };
-        let Some(predicate) = bind_stats(predicate, &mut binder)? else {
-            vortex_bail!("missing stats should lower to null literals");
-        };
-        Ok(predicate)
+        bind_stats(predicate, &mut binder)
     }
 
     pub fn file_stats(&self) -> &FileStatistics {
@@ -141,6 +137,10 @@ impl StatBinder for FileStatsBinder<'_> {
         self.reader.child.dtype()
     }
 
+    fn bound_scope(&self) -> DType {
+        DType::Null
+    }
+
     fn bind_stat(
         &mut self,
         input: &Expression,

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

@@ -121,10 +121,7 @@ impl ZoneMap {
 
     fn lower_stats(&self, predicate: Expression) -> VortexResult<Expression> {
         let mut binder = ZoneMapStatsBinder { zone_map: self };
-        let Some(predicate) = bind_stats(predicate, &mut binder)? else {
-            vortex_bail!("missing stats should lower to null literals");
-        };
-        Ok(predicate)
+        bind_stats(predicate, &mut binder)
     }
 }
 
@@ -137,6 +134,10 @@ impl StatBinder for ZoneMapStatsBinder<'_> {
         &self.zone_map.column_dtype
     }
 
+    fn bound_scope(&self) -> DType {
+        self.zone_map.array.dtype().clone()
+    }
+
     fn bind_stat(
         &mut self,
         input: &Expression,