Skip to content

Commit 0f7bddd

Browse files
authored
fix(common): preserve an exact zero through filter selectivity estimation (#23936)
## Which issue does this PR close? - Part of #8227. ## Rationale for this change `Precision::with_estimated_selectivity` always demoted to inexact, so `Exact(0)` became `Inexact(0)`. Scaling zero by any selectivity is still exactly zero: filtering a provably empty input cannot produce rows. Because operators widen exact to inexact, emptiness could not propagate up a plan through statistics at all, it was lost at the first `FilterExec`. That operator already preserved the exact zero for a contradictory predicate and for column byte sizes, so the same fact came out exact or inexact depending on the path taken. This is the same reasoning as #23670, which stops `FileScanConfig::statistics()` demoting an exact zero when a filter is present, applied one level up. The two are independent, but together they let a proof of emptiness survive from the scan through the filter. ## What changes are included in this PR? - `with_estimated_selectivity` returns an exact zero unchanged. `Inexact(0)` is untouched: an estimate of zero is not a proof of zero. - `scale_byte_size` in `filter.rs` is removed, subsumed by the above. ## Are these changes tested? Yes. A unit test for the method, and a `FilterExec` test that a satisfiable predicate over an exactly empty input keeps `num_rows`, `total_byte_size` (proving that `scale_byte_size` can be safely removed) and column `byte_size` exact. `test_filter_statistics_empty_input_equality_ndv_zero` asserted `Inexact(0)` and now asserts `Exact(0)`, which is expected. ## Are there any user-facing changes? No breaking changes. `Precision::with_estimated_selectivity` is public and keeps its signature, but its documented contract changes: it previously stated it would always return inexact statistics, and it now preserves an exact zero. Callers relying on the old wording will see `Exact(0)` where they saw `Inexact(0)`. This is an improvement and could be arguably considered a bug-fix, so I consider this contract change justified. `FilterExec` over a provably empty input therefore reports exact statistics, visible in `EXPLAIN` output that shows statistics. ---- Disclaimer: I used AI to assist in the code generation, I have manually reviewed the output and it matches my intention and understanding.
1 parent 95226ac commit 0f7bddd

2 files changed

Lines changed: 86 additions & 16 deletions

File tree

datafusion/common/src/stats.rs

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,12 @@ impl Precision<usize> {
195195
/// Return the estimate of applying a filter with estimated selectivity
196196
/// `selectivity` to this Precision. A selectivity of `1.0` means that all
197197
/// rows are selected. A selectivity of `0.5` means half the rows are
198-
/// selected. Will always return inexact statistics.
198+
/// selected. An exact zero is preserved, since filtering an empty input
199+
/// cannot produce rows; any other known value is demoted to inexact.
199200
pub fn with_estimated_selectivity(self, selectivity: f64) -> Self {
201+
if self == Precision::Exact(0) {
202+
return self;
203+
}
200204
self.map(|v| ((v as f64 * selectivity).ceil()) as usize)
201205
.to_inexact()
202206
}
@@ -1202,6 +1206,44 @@ mod tests {
12021206
assert_eq!(absent_precision.get_value(), None);
12031207
}
12041208

1209+
#[test]
1210+
fn test_with_estimated_selectivity() {
1211+
// Filtering an empty input cannot produce rows, so the zero stays exact.
1212+
assert_eq!(
1213+
Precision::Exact(0).with_estimated_selectivity(0.5),
1214+
Precision::Exact(0)
1215+
);
1216+
assert_eq!(
1217+
Precision::Exact(0).with_estimated_selectivity(1.0),
1218+
Precision::Exact(0)
1219+
);
1220+
1221+
// Any other known value is scaled and demoted, since the selectivity is
1222+
// itself an estimate.
1223+
assert_eq!(
1224+
Precision::Exact(100).with_estimated_selectivity(0.5),
1225+
Precision::Inexact(50)
1226+
);
1227+
assert_eq!(
1228+
Precision::Exact(100).with_estimated_selectivity(1.0),
1229+
Precision::Inexact(100)
1230+
);
1231+
assert_eq!(
1232+
Precision::Exact(3).with_estimated_selectivity(0.5),
1233+
Precision::Inexact(2)
1234+
);
1235+
1236+
// An inexact zero is an estimate, not a proof, and stays inexact.
1237+
assert_eq!(
1238+
Precision::Inexact(0).with_estimated_selectivity(0.5),
1239+
Precision::Inexact(0)
1240+
);
1241+
assert_eq!(
1242+
Precision::<usize>::Absent.with_estimated_selectivity(0.5),
1243+
Precision::Absent
1244+
);
1245+
}
1246+
12051247
#[test]
12061248
fn test_map() {
12071249
let exact_precision = Precision::Exact(42);

datafusion/physical-plan/src/filter.rs

Lines changed: 43 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,8 @@ impl FilterExec {
384384
input_num_rows.with_estimated_selectivity(selectivity);
385385
let mut cs = input_stats.to_inexact().column_statistics;
386386
for (idx, col_stat) in cs.iter_mut().enumerate() {
387-
col_stat.byte_size = scale_byte_size(col_stat.byte_size, selectivity);
387+
col_stat.byte_size =
388+
col_stat.byte_size.with_estimated_selectivity(selectivity);
388389
col_stat.null_count = if null_rejecting_columns.contains(&idx) {
389390
Precision::Exact(0)
390391
} else {
@@ -1030,16 +1031,6 @@ fn interval_bound_to_precision(
10301031
}
10311032
}
10321033

1033-
/// Scales a column's `byte_size` by the estimated filter `selectivity`. An
1034-
/// exact zero is preserved: an empty column stays exactly empty after
1035-
/// filtering.
1036-
fn scale_byte_size(byte_size: Precision<usize>, selectivity: f64) -> Precision<usize> {
1037-
match byte_size {
1038-
Precision::Exact(0) => Precision::Exact(0),
1039-
byte_size => byte_size.with_estimated_selectivity(selectivity),
1040-
}
1041-
}
1042-
10431034
/// Caps a row-bounded column statistic (a null count or distinct count) at the
10441035
/// filtered row estimate, since a column cannot have more nulls or distinct
10451036
/// values than it has rows. Known counts are demoted to inexact because the
@@ -1133,8 +1124,9 @@ fn collect_new_statistics(
11331124
} else {
11341125
cap_at_rows(input_column_stats[idx].null_count, filtered_num_rows)
11351126
};
1136-
let byte_size =
1137-
scale_byte_size(input_column_stats[idx].byte_size, selectivity);
1127+
let byte_size = input_column_stats[idx]
1128+
.byte_size
1129+
.with_estimated_selectivity(selectivity);
11381130
ColumnStatistics {
11391131
null_count: capped_null_count,
11401132
max_value,
@@ -2909,6 +2901,42 @@ mod tests {
29092901
Ok(())
29102902
}
29112903

2904+
#[tokio::test]
2905+
async fn test_filter_statistics_preserves_exactly_empty_input() -> Result<()> {
2906+
// A satisfiable predicate over an exactly empty input: the filter cannot
2907+
// produce rows, so the whole estimate stays exact.
2908+
let schema = Schema::new(vec![Field::new("a", DataType::Int32, true)]);
2909+
let input_stats = Statistics {
2910+
num_rows: Precision::Exact(0),
2911+
total_byte_size: Precision::Exact(0),
2912+
column_statistics: vec![ColumnStatistics {
2913+
null_count: Precision::Exact(0),
2914+
byte_size: Precision::Exact(0),
2915+
..Default::default()
2916+
}],
2917+
};
2918+
let predicate = Arc::new(BinaryExpr::new(
2919+
Arc::new(Column::new("a", 0)),
2920+
Operator::Gt,
2921+
Arc::new(Literal::new(ScalarValue::Int32(Some(5)))),
2922+
));
2923+
2924+
let input = Arc::new(StatisticsExec::new(input_stats, schema));
2925+
let filter: Arc<dyn ExecutionPlan> =
2926+
Arc::new(FilterExec::try_new(predicate, input)?);
2927+
let statistics =
2928+
StatisticsContext::new().compute(filter.as_ref(), &StatisticsArgs::new())?;
2929+
2930+
assert_eq!(statistics.num_rows, Precision::Exact(0));
2931+
assert_eq!(statistics.total_byte_size, Precision::Exact(0));
2932+
assert_eq!(
2933+
statistics.column_statistics[0].byte_size,
2934+
Precision::Exact(0)
2935+
);
2936+
2937+
Ok(())
2938+
}
2939+
29122940
#[tokio::test]
29132941
async fn test_filter_statistics_empty_input_equality_ndv_zero() -> Result<()> {
29142942
let cases: Vec<(&str, Schema, Statistics, Arc<dyn PhysicalExpr>)> = vec![
@@ -2963,12 +2991,12 @@ mod tests {
29632991

29642992
assert_eq!(
29652993
statistics.num_rows,
2966-
Precision::Inexact(0),
2994+
Precision::Exact(0),
29672995
"case '{desc}': row count mismatch"
29682996
);
29692997
assert_eq!(
29702998
statistics.column_statistics[0].distinct_count,
2971-
Precision::Inexact(0),
2999+
Precision::Exact(0),
29723000
"case '{desc}': NDV should be capped at zero rows"
29733001
);
29743002
}

0 commit comments

Comments
 (0)