github-actions[bot] commented on code in PR #67940:
URL: https://github.com/apache/doris/pull/67940#discussion_r4006492039
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/UnCorrelatedApplyAggregateFilter.java:
##########
@@ -64,28 +104,95 @@
* |
* Filter(Uncorrelated predicate)
* </pre>
+ * <p>
+ * The rewrite above keeps the aggregation of the subquery on the inner side,
which is only
+ * equivalent to the original subquery when the correlated predicate is an
equality between the
+ * outer side and the inner side: in that case the inner rows of one outer row
are exactly the
+ * groups of the aggregate whose key is the value of that inner side, so the
HAVING clause of every
+ * group is the HAVING clause of the outer row.
+ * The aggregation of an EXISTS/NOT EXISTS subquery is built on the outer side
instead when that
+ * equivalence does not hold, see
+ * {@link #pullUpCorrelatedPredicateByAggregatingOuter}.
*/
public class UnCorrelatedApplyAggregateFilter implements RewriteRuleFactory {
+
+ /** name of the projected column which tells whether an inner row matched
the correlated predicate */
+ private static final String CORRELATION_MATCH_MARKER =
"$correlation_match_marker";
+
@Override
public List<Rule> buildRules() {
+ // The nodes between the apply and the aggregate of the subquery are
projections and filters
+ // in any order, so the rules cannot enumerate their shapes: match
every correlated apply
+ // whose right side starts with one of them and locate the aggregate
in the rule.
return ImmutableList.of(
- logicalApply(any(), logicalAggregate(logicalFilter()))
+ logicalApply(any(), subTree(LogicalAggregate.class,
LogicalProject.class, LogicalFilter.class))
Review Comment:
[P1] Keep the newly matched IN wrappers resolvable. A legal IN right side
can remain `Project(c) -> Filter(r < 0.5) -> Project(c, random() AS r) ->
Aggregate(count(*) AS c) -> Filter(inner.k = outer.k)`: volatile-alias filter
pushdown leaves the inner project in place, and `PullUpProjectUnderApply`
retains the root IN project whose child is a filter. This widened matcher now
admits that chain, but the non-EXISTS legacy rewrite adds `inner.k` only to the
aggregate and to `apply.correlationFilter`; `replaceAggregate` rebuilds both
projects without exposing it. `InApplyToJoin` then creates a join predicate
containing an ExprId absent from the right child's output, so
`CheckAfterRewrite` rejects the plan. Please expose the added key through
retained projects or explicitly reject/restrict this newly admitted IN shape,
and add an IN resolvability test.
--
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]
