kumarUjjawal commented on code in PR #26044:
URL: https://github.com/apache/datafusion/pull/26044#discussion_r4203682059


##########
datafusion/optimizer/src/decorrelate.rs:
##########
@@ -828,6 +859,36 @@ fn can_pullup_over_aggregation(expr: &Expr) -> bool {
     }
 }
 
+/// Rewrite columns in `filters` that are listed in `cols` so they use `alias`.
+///
+/// `cols` are the correlated columns as named below the alias. Outer
+/// references are not in that set, so they stay as they are. The caller
+/// passes only the filters added under this alias.
+fn requalify_join_filter_columns(
+    filters: &mut [Expr],
+    cols: &BTreeSet<Column>,
+    alias: &TableReference,
+) -> Result<()> {
+    if cols.is_empty() || filters.is_empty() {
+        return Ok(());
+    }
+    for filter in filters {
+        *filter = filter
+            .clone()
+            .transform(|expr| {
+                if let Expr::Column(col) = &expr
+                    && cols.contains(col)
+                {
+                    let new_col = Column::new(Some(alias.clone()), 
col.name.clone());

Review Comment:
   Keeping col.name is incorrect when pull-up adds a correlation column whose 
name matches another projected column. SubqueryAlias::try_new disambiguates 
duplicate field names using suffixes such as id:1, so changing only the 
qualifier can target a different column.
   
   Use the rebuilt alias’s actual input-to-output column mapping for both the 
filters and propagated correlation columns, and add a collision regression case.



-- 
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]

Reply via email to