peter-toth commented on PR #58552: URL: https://github.com/apache/spark/pull/58552#issuecomment-5618958093
All six taken, thank you - [`72919b8`](https://github.com/apache/spark/commit/72919b80ccaca955b024658cda277d70a1a12648). **One of them was a regression this PR introduced, and you read it exactly right.** The base cleared the marker with `k.copy(mayContainUnknownPartitionKeys = false)`, which kept the `partitionKeys` reference, so `intern` returned both sides untouched. Holding the marker in `KeyLayout` means clearing it has to build a layout, and a fresh one is never `eq` the unmarked side's, so one side rebuilt and re-checked the collection invariant on every `outputPartitioning` call. `ShuffledJoin` now reuses the unmarked side's layout object when it equals the cleared copy, which is the common inner-join shape, and keeps the fresh copy as the fallback. The other five are as you described them: - `ShuffledJoin` computes the layout once from the representative, so the `var` and the guard the outer marker check already made dead both go. `representativeOf` is `private[sql]` for it. - `KeyLayout.describesSameKeys` holds the key comparison and its rationale once, called from `intern` and from `KeyedShuffleSpec.isCompatibleWith`. `isGrouped` stays separate and the scaladoc says why. - `PartitionGrouping` holds a `KeyLayout` instead of three of its fields, built where the child is in scope, so `outputPartitioning` no longer re-applies the marker. - `KeyedPartitioning.apply` loses `sortKeys`; the scan sorts with `groupedKeyRowOrdering`, which names the shared contract instead of carrying a boolean. - `createPartitioning` builds one node. One knock-on worth flagging: the `SPARK-46367` test asserts on the word `partitionKeys` in the invariant message, which the consolidated `require` no longer printed. The message names both fields now rather than only their values. 458 tests green, lint clean. -- 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]
