yadavay-amzn commented on PR #56961: URL: https://github.com/apache/spark/pull/56961#issuecomment-4888337617
### **LGTM, one test-coverage suggestion (non-blocking)** Traced the merge end to end and the core is sound - the partition-index-to-key alignment holds: `sparkContext.union` concatenates children in order, `mergedKeys` concatenates `partitionKeys` in the same child order, and each child's `DataSourceRDD` keeps one partition per key (pruned splits stay as `None` placeholders in `filteredPartitions`, so a child's partition count always equals its `partitionKeys.length`). So the descriptor and the RDD layout stay aligned. Building on peter-toth's points, not repeating them. One gap worth closing: none of the added tests exercise a union child whose partitions are all pruned (an empty / `None`-padded leg), and because every current case is order-insensitive, no test would fail if that index-to-key alignment were ever broken. A case where one union leg's keys are entirely runtime-pruned (or a leg contributes zero live partitions) while the merged `KeyedPartitioning` still carries its keys would directly guard the concatenation invariant the shuffle-free path depends on. What I verified: - `mergedKeys` (`basicPhysicalOperators.scala:957`) and `sparkContext.union` (`:1200`) both concatenate in children order, so `mergedKeys(i)` describes union RDD partition `i`. - `filteredPartitions: Seq[Option[InputPartition]]` (`BatchScanExec.scala:79`) pads pruned splits with `None` rather than dropping them, so per-child partition count == `partitionKeys.length` even after runtime filtering - the alignment holds under pruning. - The `KeyedPartitioning` `doExecute` branch correctly avoids `SQLPartitioningAwareUnionRDD` (index-interleaving would mix keys); plain concat matches the merged descriptor. -- 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]
