xudong963 commented on code in PR #23395:
URL: https://github.com/apache/datafusion/pull/23395#discussion_r3549085883
##########
datafusion/datasource/src/file_scan_config/mod.rs:
##########
@@ -649,6 +650,45 @@ fn project_output_partitioning(
}
}
+/// Returns `true` if merging `outer` into `inner` would duplicate a volatile
+/// expression; the caller should then decline the merge.
+///
+/// `inner` is the scan's current projection and `outer` the projection being
+/// pushed into it; merging substitutes each `inner` expression into every
+/// `outer` reference to it. If a volatile `inner` expression (e.g. `random()`,
+/// `uuid()`) is referenced by `outer`, that one value gets inlined at each
site
+/// and re-evaluated independently, so references meant to share a single
+/// "locked-in" value diverge. This is the volatility guard the physical
+/// `ProjectionPushdown` and `FilterPushdown` rules already apply (see
+/// `datafusion_physical_expr_common::physical_expr::is_volatile`).
+///
+/// A volatile `inner` expression blocks the merge whenever `outer` references
+/// it at all: `collect_columns` reports a column at most once per `outer`
+/// expression, so a self-duplicating expression such as `r + r` would slip
past
+/// a stricter `> 1` threshold.
+fn would_duplicate_volatile_exprs(
+ inner: &ProjectionExprs,
+ outer: &ProjectionExprs,
+) -> bool {
+ let inner_exprs = inner.as_ref();
+ let outer_exprs = outer.as_ref();
+
+ let mut referenced = vec![false; inner_exprs.len()];
+ for proj_expr in outer_exprs {
+ for col in collect_columns(&proj_expr.expr) {
+ let idx = col.index();
+ if idx < referenced.len() {
+ referenced[idx] = true;
+ }
+ }
+ }
+
+ referenced
+ .iter()
+ .enumerate()
+ .any(|(idx, &referenced)| referenced &&
is_volatile(&inner_exprs[idx].expr))
Review Comment:
That is safe but imprecise. A better implementation would count references
with `expr.apply(...)`, like `try_collapse_projection_chain` already does and
block only when a volatile inner expression is referenced more than once.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
