feat: extend interval arithmetic to support scalar functions (UDFs) - #22248
feat: extend interval arithmetic to support scalar functions (UDFs)#22248davidlghellin wants to merge 23 commits into
Conversation
…analysis `check_support` previously returned `false` for any `ScalarFunctionExpr`, preventing `FilterExec::statistics()` from entering the interval-analysis path for predicates containing scalar UDFs (e.g. `ceil(x) > 100`). Extend `check_support` to recurse into `ScalarFunctionExpr` when both the return type and all argument expressions are supported. This enables `evaluate_bounds` and `propagate_constraints` on `ScalarUDFImpl` to participate in selectivity estimation and constraint propagation.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR extends interval-analysis support to include ScalarFunctionExpr (notably ceil) so filter selectivity/statistics can be estimated more accurately, and adds tests covering the new behavior.
Changes:
- Add
ScalarFunctionExprhandling tocheck_supportso interval analysis can traverse scalar functions. - Implement bounds evaluation and constraint propagation for
CeilFunc. - Add regression tests for both
check_supportand filter statistics withceil(x) > const.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| datafusion/physical-plan/src/filter.rs | Adds a regression test ensuring filter statistics are narrowed when predicates include ceil(x). |
| datafusion/physical-expr/src/intervals/utils.rs | Extends check_support to allow scalar functions and adds unit tests covering support detection. |
| datafusion/functions/src/math/ceil.rs | Implements evaluate_bounds and propagate_constraints for ceil to enable interval narrowing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let num_rows = statistics.num_rows.get_value().copied().unwrap_or(100); | ||
| // Interval analysis must narrow the estimate below the full 100-row input. | ||
| assert!( | ||
| num_rows < 100, | ||
| "expected interval analysis to narrow row estimate, got {num_rows}" | ||
| ); | ||
| // The conservative bound is x ∈ [11, 16] out of [8, 16] → ~62 rows. | ||
| // Allow a generous range to be robust to float-cardinality rounding. | ||
| assert!( | ||
| num_rows >= 50, | ||
| "expected at least 50 rows after ceil(x) > 12.0 on [8,16], got {num_rows}" |
- Revert the unrelated SkipAggregationProbe `>` comment in skip_partial.rs back to upstream (partial-aggregation skip is out of scope for this change). - Document ScalarFunctionExpr support in the `check_support` doc comment.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #22248 +/- ##
==========================================
+ Coverage 80.86% 80.88% +0.02%
==========================================
Files 1101 1101
Lines 375446 375965 +519
Branches 375446 375965 +519
==========================================
+ Hits 303588 304108 +520
+ Misses 53766 53759 -7
- Partials 18092 18098 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Which issue does this PR close?
Rationale for this change
FilterExec::statistics()uses interval arithmetic to narrow row-count estimates. The entry gate for this analysis ischeck_support, which decides whether an expression can participate. Previously, ScalarFunctionExpr was not recognized bycheck_support, so any filter containing a scalar UDF call (e.g.ceil(x) > 12) would skip interval analysis entirely and fall back to a flat 50% selectivity guess, even if the UDF implementedevaluate_boundsandpropagate_constraints.What changes are included in this PR?
datafusion/physical-expr/src/intervals/utils.rs: extendcheck_supportto recognizeScalarFunctionExpr, gating on whether the return type and all argument expressions are supported.datafusion/functions/src/math/ceil.rs: implementevaluate_boundsandpropagate_constraintsforCeilFuncas a working example of a UDF that benefits from the fix.Are these changes tested?
Yes:
5 unit tests in
intervals/utils.rscovering supported/unsupported return types, unsupported child expressions, andScalarFunctionExprinside aBinaryExpr.1 integration test in
filter.rs(test_filter_statistics_ceil_scalar_fn) that verifiesFilterExec::partition_statisticsproduces a narrowed row estimate forceil(x) > 12.0over a column with known min/max bounds.Are there any user-facing changes?
No API changes. The improvement is visible as more accurate row estimates in query plans containing scalar UDF filters.
related lakehq/sail#1917