CuteChuanChuan commented on code in PR #27133:
URL: https://github.com/apache/datafusion/pull/27133#discussion_r4553636002
##########
datafusion/optimizer/src/decorrelate.rs:
##########
@@ -573,10 +573,15 @@ impl TreeNodeRewriter for PullUpCorrelatedExpr {
self.can_pull_up = false;
return Ok(Transformed::new(plan, false,
TreeNodeRecursion::Stop));
}
- // If the aggregation is from a distinct it will not change
the result for
- // exists/in subqueries so we can still pull up all predicates.
+ // Only a DISTINCT under EXISTS/IN may skip this check. The
pulled
+ // up column becomes an extra group key, which for a DISTINCT
only
+ // adds duplicate rows, and EXISTS/IN ignore duplicates.
Elsewhere,
+ // e.g. LATERAL, the duplicates reach the result. See #25808.
let is_distinct = aggregate.aggr_expr.is_empty();
- if !is_distinct {
+ let is_exists_or_in =
+ self.exists_sub_query || self.in_predicate_opt.is_some();
+ let can_skip_check = is_distinct && is_exists_or_in;
+ if !can_skip_check {
self.can_pull_up = self.can_pull_up &&
self.can_pull_over_aggregation;
}
Review Comment:
Thanks for pointing this out. I only considered whether duplicates matter,
not whether they're created at all. Addressed in 5eed032.
--
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]
