Skip to content

Commit 6aaf396

Browse files
xudong963claude
xudong963
and
claude
committed
Make collect_predicate_constants a method on ConstExpr for discoverability
Address alamb's review comment: move `collect_predicate_constants` from a standalone function to `ConstExpr::collect_predicate_constants()` and add a textual example to the doc comment. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 5ab6e6e commit 6aaf396

3 files changed

Lines changed: 58 additions & 52 deletions

File tree

‎datafusion/physical-expr/src/lib.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,7 @@ pub use datafusion_physical_expr_common::sort_expr::{
7272
pub use planner::{create_physical_expr, create_physical_exprs};
7373
pub use scalar_function::ScalarFunctionExpr;
7474
pub use simplifier::PhysicalExprSimplifier;
75-
pub use utils::{
76-
collect_predicate_constants, conjunction, conjunction_opt, split_conjunction,
77-
};
75+
pub use utils::{conjunction, conjunction_opt, split_conjunction};
7876

7977
// For backwards compatibility
8078
pub mod tree_node {

‎datafusion/physical-expr/src/utils/mod.rs‎

Lines changed: 55 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -46,57 +46,64 @@ pub fn split_conjunction(
4646
split_impl(Operator::And, predicate, vec![])
4747
}
4848

49-
/// Collects predicate-derived constants from equality conjunctions.
50-
///
51-
/// For each equality predicate of the form `lhs = rhs`, if either side is
52-
/// already known constant according to `input_eqs`, or is a literal, then the
53-
/// other side is also constant and will be returned as a [`ConstExpr`].
54-
///
55-
/// Literals are treated as uniform constants across partitions, so
56-
/// `col = literal` produces a constant for `col` with the literal value.
57-
pub fn collect_predicate_constants(
58-
input_eqs: &EquivalenceProperties,
59-
predicate: &Arc<dyn PhysicalExpr>,
60-
) -> Vec<ConstExpr> {
61-
/// Returns the `AcrossPartitions` value for `expr` if it is constant:
62-
/// either already known constant in `input_eqs`, or a `Literal`
63-
/// (which is inherently constant across all partitions).
64-
fn expr_constant_or_literal(
65-
expr: &Arc<dyn PhysicalExpr>,
49+
impl ConstExpr {
50+
/// Collects predicate-derived constants from equality conjunctions.
51+
///
52+
/// For each equality predicate of the form `lhs = rhs`, if either side is
53+
/// already known constant according to `input_eqs`, or is a literal, then
54+
/// the other side is also constant and will be returned as a [`ConstExpr`].
55+
///
56+
/// Literals are treated as uniform constants across partitions, so
57+
/// `col = literal` produces a constant for `col` with the literal value.
58+
///
59+
/// For example, given predicate `a = 5 AND b = c` where `c` is already
60+
/// known constant, this returns constants for both `a` (Uniform with value
61+
/// 5) and `b` (propagating `c`'s across-partitions value).
62+
pub fn collect_predicate_constants(
6663
input_eqs: &EquivalenceProperties,
67-
) -> Option<AcrossPartitions> {
68-
input_eqs.is_expr_constant(expr).or_else(|| {
69-
expr.as_any()
70-
.downcast_ref::<Literal>()
71-
.map(|l| AcrossPartitions::Uniform(Some(l.value().clone())))
72-
})
73-
}
64+
predicate: &Arc<dyn PhysicalExpr>,
65+
) -> Vec<ConstExpr> {
66+
/// Returns the `AcrossPartitions` value for `expr` if it is constant:
67+
/// either already known constant in `input_eqs`, or a `Literal`
68+
/// (which is inherently constant across all partitions).
69+
fn expr_constant_or_literal(
70+
expr: &Arc<dyn PhysicalExpr>,
71+
input_eqs: &EquivalenceProperties,
72+
) -> Option<AcrossPartitions> {
73+
input_eqs.is_expr_constant(expr).or_else(|| {
74+
expr.as_any()
75+
.downcast_ref::<Literal>()
76+
.map(|l| AcrossPartitions::Uniform(Some(l.value().clone())))
77+
})
78+
}
7479

75-
let mut constants = Vec::new();
76-
for conjunction in split_conjunction(predicate) {
77-
if let Some(binary) = conjunction.as_any().downcast_ref::<BinaryExpr>()
78-
&& binary.op() == &Operator::Eq
79-
{
80-
// Check if either side is constant — either already known
81-
// constant from the input equivalence properties, or a literal
82-
// value (which is inherently constant across all partitions).
83-
let left_const = expr_constant_or_literal(binary.left(), input_eqs);
84-
let right_const = expr_constant_or_literal(binary.right(), input_eqs);
85-
86-
if let Some(left_across) = left_const {
87-
// LEFT is constant, so RIGHT must also be constant.
88-
// Use RIGHT's known across value if available, otherwise
89-
// propagate LEFT's (e.g. Uniform from a literal).
90-
let across = right_const.unwrap_or(left_across);
91-
constants.push(ConstExpr::new(Arc::clone(binary.right()), across));
92-
} else if let Some(right_across) = right_const {
93-
// RIGHT is constant, so LEFT must also be constant.
94-
constants.push(ConstExpr::new(Arc::clone(binary.left()), right_across));
80+
let mut constants = Vec::new();
81+
for conjunction in split_conjunction(predicate) {
82+
if let Some(binary) = conjunction.as_any().downcast_ref::<BinaryExpr>()
83+
&& binary.op() == &Operator::Eq
84+
{
85+
// Check if either side is constant — either already known
86+
// constant from the input equivalence properties, or a literal
87+
// value (which is inherently constant across all partitions).
88+
let left_const = expr_constant_or_literal(binary.left(), input_eqs);
89+
let right_const = expr_constant_or_literal(binary.right(), input_eqs);
90+
91+
if let Some(left_across) = left_const {
92+
// LEFT is constant, so RIGHT must also be constant.
93+
// Use RIGHT's known across value if available, otherwise
94+
// propagate LEFT's (e.g. Uniform from a literal).
95+
let across = right_const.unwrap_or(left_across);
96+
constants.push(ConstExpr::new(Arc::clone(binary.right()), across));
97+
} else if let Some(right_across) = right_const {
98+
// RIGHT is constant, so LEFT must also be constant.
99+
constants
100+
.push(ConstExpr::new(Arc::clone(binary.left()), right_across));
101+
}
95102
}
96103
}
97-
}
98104

99-
constants
105+
constants
106+
}
100107
}
101108

102109
/// Create a conjunction of the given predicates.
@@ -629,7 +636,8 @@ pub(crate) mod tests {
629636
)?;
630637
let eq_properties = EquivalenceProperties::new(schema);
631638

632-
let constants = collect_predicate_constants(&eq_properties, &predicate);
639+
let constants =
640+
ConstExpr::collect_predicate_constants(&eq_properties, &predicate);
633641

634642
assert_eq!(constants.len(), 1);
635643
assert_eq!(

‎datafusion/physical-plan/src/filter.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ use datafusion_physical_expr::intervals::utils::check_support;
6464
use datafusion_physical_expr::utils::{collect_columns, reassign_expr_columns};
6565
use datafusion_physical_expr::{
6666
AcrossPartitions, AnalysisContext, ConstExpr, ExprBoundaries, PhysicalExpr, analyze,
67-
collect_predicate_constants, conjunction, split_conjunction,
67+
conjunction, split_conjunction,
6868
};
6969

7070
use datafusion_physical_expr_common::physical_expr::fmt_sql;
@@ -387,7 +387,7 @@ impl FilterExec {
387387
eq_properties.add_constants(constants)?;
388388
// This is for logical constant (for example: a = '1', then a could be marked as a constant)
389389
// to do: how to deal with multiple situation to represent = (for example c1 between 0 and 0)
390-
eq_properties.add_constants(collect_predicate_constants(
390+
eq_properties.add_constants(ConstExpr::collect_predicate_constants(
391391
input.equivalence_properties(),
392392
predicate,
393393
))?;

0 commit comments

Comments
 (0)