rubenada commented on code in PR #4614:
URL: https://github.com/apache/calcite/pull/4614#discussion_r2498214932
##########
core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java:
##########
@@ -804,17 +804,18 @@ protected RexNode removeCorrelationExpr(
}
}
+ final Correlate correlate = frameStack.peek() == null ? null :
frameStack.peek().left;
if (rel.getGroupType() == Aggregate.Group.SIMPLE
&& rel.getGroupSet().isEmpty()
&& !frame.corDefOutputs.isEmpty()
&& (
// If there is a COUNT, we must call rewriteScalarAggregate (to
distinguish 0 vs empty)
hasCount
||
- // Otherwise call only for non-LEFT Correlate, because for LEFT:
NULL is effectively
- // the same as empty (which promotes NULL on the RHS)
+ // Otherwise call except if this is a LEFT Correlate with the
Aggregate being its RHS,
+ // in that case NULL is effectively the same as empty (which
promotes NULL on the RHS)
(!parentPropagatesNullValues
- && requireNonNull(frameStack.peek()).left.getJoinType() !=
JoinRelType.LEFT))) {
Review Comment:
You mean on `decorrelateRel(Correlate)` ?
That actually seems to do the trick in a much cleaner way. It maintains the
plans adjusted in the PR, fixes the counter-example that you proposed on my
initial commit, and it also works as expected on my downstream project's tests
if I apply it.
I've pushed this change, cleaning up the previous modifications.
--
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]
