dongjoon-hyun commented on code in PR #58758:
URL: https://github.com/apache/spark/pull/58758#discussion_r4001846566
##########
core/src/main/scala/org/apache/spark/rdd/PairRDDFunctions.scala:
##########
@@ -1111,4 +1103,10 @@ class PairRDDFunctions[K, V](self: RDD[(K, V)])
private[spark] def valueClass: Class[_] = vt.runtimeClass
private[spark] def keyOrdering: Option[Ordering[K]] = Option(ord)
+
+ private def failOnHashPartitionerWithArrayKey(partitioner: Partitioner):
Unit = {
Review Comment:
`combineByKeyWithClassTag` (L80-87) still has a fifth copy of this check. It
can reuse this helper while keeping the `MAP_SIDE_COMBINE`-first order that
`PartitioningSuite` relies on:
```scala
if (keyClass.isArray && mapSideCombine) {
throw SparkCoreErrors.cannotUseMapSideCombiningWithArrayKeyError()
}
failOnHashPartitionerWithArrayKey(partitioner)
```
This would complete the "keep the rule in one place" goal and route
`reduceByKey`, `groupByKey`, `aggregateByKey`, and `foldByKey` through the
helper as well.
##########
core/src/main/scala/org/apache/spark/rdd/PairRDDFunctions.scala:
##########
@@ -778,9 +776,7 @@ class PairRDDFunctions[K, V](self: RDD[(K, V)])
other3: RDD[(K, W3)],
partitioner: Partitioner)
: RDD[(K, (Iterable[V], Iterable[W1], Iterable[W2], Iterable[W3]))] =
self.withScope {
- if (partitioner.isInstanceOf[HashPartitioner] && keyClass.isArray) {
- throw SparkCoreErrors.hashPartitionerCannotPartitionArrayKeyError()
- }
+ failOnHashPartitionerWithArrayKey(partitioner)
Review Comment:
The 3-way and 4-way `cogroup(..., partitioner)` guards are not covered by
`PartitioningSuite` ("partitioning Java arrays should fail" only exercises
`partitionBy` and the 2-way `cogroup` via `join`, `*OuterJoin`, and `cogroup`).
Could you add the following so that all four refactored call sites are verified?
```scala
verify("HASH_PARTITIONER")(arrPairs.cogroup(arrPairs, arrPairs))
verify("HASH_PARTITIONER")(arrPairs.cogroup(arrPairs, arrPairs, arrPairs))
```
--
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]
