cloud-fan commented on PR #58144:
URL: https://github.com/apache/spark/pull/58144#issuecomment-6135733232
Thanks for the update. I rechecked the current head: the lower-bound
qualification, the complete-ordering sort grouping, and the
nondeterministic-window, nondeterministic-link and reversed-key tests look
addressed. Two items from the last round are still open, though.
1. When a rank filter pins the top window, reorderChain still calls
optimalOrder(windows.dropRight(1)) without the pinned window's partition spec.
For example, lower windows [a], [b] under a pinned rank window [a] keep 3
exchanges, while [b], [a], pinned [a] needs only 2. Could you pass the pinned
spec to optimalOrder as a tail constraint, so that a group the pinned window
can ride is scheduled last? If several groups qualify, keeping the one that
already appears last keeps the result a fixed point.
2. The new two-link test does not exercise refsAbove: its upper link passes
every attribute it references through to the chain output, so the test still
passes with refsAbove emptied. A case where the upper link computes an alias
from the output of the window between the two links, and drops that output,
would cover it.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"conversation:6099761026","thread_id":"conversation:6099761026","verdict_sha256":"b33b75c7b5ce87d7eac711e93e22ea31c3718cfd6efdb8d9cc565b92c1cf4715"}
-->
--
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]
