mohammadnaqvi04 commented on code in PR #25391:
URL: https://github.com/apache/datafusion/pull/25391#discussion_r4328550044


##########
datafusion/optimizer/src/decorrelate.rs:
##########
@@ -677,31 +749,80 @@ impl PullUpCorrelatedExpr {
     }
 
     fn collect_missing_exprs(
-        &self,
+        &mut self,
         exprs: &[Expr],
         correlated_subquery_cols: &BTreeSet<Column>,
+        subquery_schema: &DFSchemaRef,
     ) -> Result<Vec<Expr>> {
         let mut missing_exprs = vec![];
         for expr in exprs {
             if !missing_exprs.contains(expr) {
                 missing_exprs.push(expr.clone())
             }
         }
+        // A correlated column compared bare (`t1.a = t2.b`) only ever
+        // matches one row per value, so grouping the subquery's own
+        // aggregate by that column is enough. But if the join filter
+        // wraps it in an expression (`t1.a = CAST(t2.b AS INT)`), two
+        // different column values can compare equal after the cast, and
+        // grouping by the bare column would compute the aggregate once
+        // per underlying value instead of once per outer row it actually
+        // joins against. Grouping by the wrapping expression instead
+        // keeps those together, matching what the join predicate itself
+        // treats as equal.
+        let join_filter_exprs =
+            collect_subquery_join_exprs(&self.join_filters, subquery_schema)?;
         for col in correlated_subquery_cols.iter() {
-            let col_expr = Expr::Column(col.clone());
-            if !missing_exprs.contains(&col_expr) {
+            if let Some(existing_alias) = self.correlated_col_aliases.get(col) 
{
+                if !collides_with_existing(&missing_exprs, existing_alias) {
+                    missing_exprs.push(existing_alias.clone())
+                }
+                continue;
+            }
+            let wrapped = join_filter_exprs

Review Comment:
   I've added `subquery_operands_are_bare_columns` and gated `compensable` on 
it. Also added `TODO(#26014)` on the four dedup tests so the expected output 
isn't read as correct. Verified against your two repros, both now match main.



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