iwanttobepowerful commented on code in PR #4997:
URL: https://github.com/apache/calcite/pull/4997#discussion_r3369181868


##########
core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java:
##########
@@ -2487,6 +2502,57 @@ private DecorrelateRexShuttle(RelNode currentRel,
       return fieldAccess;
     }
 
+    /**
+     * Window operators are decorrelated similarly to aggregates. A correlated
+     * window expression is evaluated once per outer-row binding before
+     * decorrelation; after decorrelation, outer references are represented as
+     * ordinary input fields. Therefore, add those fields to the window 
partition
+     * keys so that the window function is still evaluated independently for
+     * each outer-row binding.
+     *
+     * <p>Implementation based on: Improving Unnesting of Complex Queries
+     *
+     * <p>3.3 Unnesting Rules
+     * 
(https://dl.gi.de/server/api/core/bitstreams/c1918e8c-6a87-4da2-930a-bfed289f2388/content)
+     */
+    @Override public RexNode visitOver(RexOver over) {

Review Comment:
   the two fixes are related but apply to different shuttles. This PR updates 
`visitOver` in `DecorrelateRexShuttle`, while #4998 updates `visitOver` in 
`RemoveCorrelationRexShuttle`.
   
   I also verified that if `RemoveCorrelationForScalarAggregateRule` is 
disabled in `removeCorrelationViaRule()`, the #4998 fix does not seem to be 
needed. So #4998 appears to address the rule-based decorrelation path, whereas 
this PR addresses the `DecorrelateRexShuttle` path.
   



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

Reply via email to