mohammadnaqvi04 commented on code in PR #25391:
URL: https://github.com/apache/datafusion/pull/25391#discussion_r4321177705
##########
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:
One thing I'd want to raise is this does reopen the CAST-collision dedup
case the alias rewrite was built for. Not just reverting to main's old wrong
answer, but in some fraction of cases producing a duplicate-row answer that's
wrong in a new way (neither DuckDB's nor main's). I differentially fuzzed this
specifically (using DuckDB as oracle) and confirmed it's isolated to
`CAST`-wrapped correlation keys only, so there're zero regressions outside that
shape.
--
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]
