peter-toth commented on PR #58814:
URL: https://github.com/apache/spark/pull/58814#issuecomment-5683160322

   Thanks @dongjoon-hyun, @ulysses-you, @uros-b. All three inline comments are 
addressed in 9f2e46aa910, with the `ShuffleSpecSuite` case you suggested.
   
   - **`@param` / `createShuffleSpec`.** You are right, my rewrite 
over-generalized. The two cases are split now: a covering projection leaves a 
usable spec, a narrowing one leaves the spec `canCreatePartitioning` and 
`areKeysCompatible` turn away, which is what the sentence I dropped said.
   - **New test.** `a marked covering projection yields a usable unprojected 
spec`, beside the narrowing one. It fails with the guard back at its 
narrowing-only form.
   - **Gate comment.** The fragment and the "would walk both sides' partition 
keys" argument are gone. That argument contradicts itself: `describesSameKeys` 
does the same walk on the push path.
   - **`SPARK-59050` comment.** Your numbering and your wording.
   
   The follow-up you describe, keeping the marked side's declared order where 
exactly one side is marked and the merged set equals its declared set, is worth 
its own ticket. I will file it.
   
   Suites and `dev/lint-scala` green.
   


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

Reply via email to