peter-toth opened a new pull request, #58501: URL: https://github.com/apache/spark/pull/58501
### What changes were proposed in this pull request? `KeyedPartitioning` gains a `keyDataTypes` field beside `expressions`, and `keyDataTypes` stops sampling the first partition key row. The types are already computed wherever the keys are built, and were dropped there. `KeyedPartitioning.apply` computed them only to build the key wrapper factory. `project` called `projectKeys(positions)._2` and threw away the projected types. `GroupPartitionsExec.grouping` computed the reduced types, `PartitionGrouping` did not carry them, and `outputPartitioning` re-derived them by sampling. All three now pass what they know. Every `copy` keeps the types, which is right at each site: `KeyedShuffleSpec.createPartitioning` and `withNewChildrenInternal` change the expressions and leave the key rows alone, and canonicalisation and `toGrouped` do not change what a key holds. Two copies do change the rows. `project` passes the projected types, and `concat`, the one place that mixes rows from several partitionings, requires the children to agree on them. The field is checked against the thing it describes rather than argued about: the constructor requires one type per expression, and requires the first key row to have been built with them. Every key wrapper carries its own types, so this costs one comparison. `PartitioningCollection`'s invariant deliberately does not gain a clause. Its members share one key list, and a wrapper compares its types before its values, so structurally equal keys force equal types, leaving only empty-key members where the types describe nothing. Two consequences in `EnsureRequirements`. The reduced-types comparison drops SPARK-59176's empty-side guard, which existed only because a keyless side reported the expressions' types. And the two `KeyedPartitioning(clustering, _, _, _)` patterns become type patterns reading `.expressions`, so a future field does not touch them. ### Why are the changes needed? `keyDataTypes` answered by reading the first key row and falling back to `expressionDataTypes` when there was none. Three things followed. A partitioning with no key reported types no key of it would have held. That is SPARK-59176, whose fix keeps such a side out of one comparison rather than giving it an answer. With the field the side answers, and the guard is gone. Nothing checked that the keys after the first agreed with it. There is now nothing to disagree with, because no key is consulted for types at all. The divergence at `KeyedShuffleSpec.createPartitioning` was accidental. `copy(expressions = ...)` silently left the types describing the old expressions. It is now an explicit property of the copy, and the scaladoc says so on `expressionDataTypes`, which is the member whose use needs the warning. ### Does this PR introduce _any_ user-facing change? No. A `stringArgs` override keeps the field out of `explain`, so plan output is unchanged. `KeyedPartitioning` is a `catalyst` class, so this is not public API. It is still a binary- and source-incompatible change to a case class: the constructor, `apply`, `copy`, `unapply` and the companion's `tupled`/`curried` all change shape. MiMa does not flag it, since `MimaExcludes` blanket-excludes `org.apache.spark.sql.catalyst.*`. The two destructuring patterns in the repository are converted to type patterns. ### How was this patch tested? A new `GroupPartitionsExecSuite` test asserts that a reduce's result type reaches the reported partitioning, both with keys and with none, while the expression the node reports keeps its own different type. It uses a both-sides reduce, which is the shape whose reported expression cannot carry the key type: `EnsureRequirements` refuses the same divergence from a one-side reduce. The existing SPARK-59176 tests pin the guard removal: both still pass with the comparison back to its plain form, including the one that asserts a contract-breaking reducer still raises `STORAGE_PARTITION_JOIN_INCOMPATIBLE_REDUCED_TYPES`. The type propagation itself is pinned by twelve existing `KeyGroupedPartitioningSuite` tests, measured by making `GroupPartitionsExec` report the child's types instead of the reduced ones. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5) -- 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]
