peter-toth commented on PR #58262: URL: https://github.com/apache/spark/pull/58262#issuecomment-5438414471
Updated and out of draft, so this is ready for another look. @dongjoon-hyun thanks for the review, all seven are addressed. The `MatchError` you found is fixed with a test that throws without it. The projection is now computed once per distinct position set rather than once per `PartitioningCollection` member, and the duplicated `PARTITION BY` test shares a fixture with the subset test above it. Your point about admission and projection being two implementations of the same question is right, and it is the next thing we want to take on in this area - it changes what `satisfies` answers for every consumer, so it did not belong in a bugfix. Since the earlier reviews the branch has also been rebased onto master twice, past SPARK-59025 and SPARK-59027, and it picked up a few things from my own re-reads: the ranking of candidate projections is now exact rather than a coverage heuristic, the "needs no node" question is asked of every member of the child's partitioning rather than only of the winner, and `requiredNumPartitions` filters the candidates rather than vetoing the winner. Nineteen tests, thirteen of which fail on master; the description has the breakdown. -- 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]
