[SPARK-48656][CORE] Do a length check and throw COLLECTION_SIZE_LIMIT_EXCEEDED error in CartesianRDD.getPartitions
#47019
Conversation
There was a problem hiding this comment.
@timlee0119 also opened a PR for this this at #47016 . Compared to that other PR, this PR has unit tests and also uses the new structured error framework / error class, so I lean towards taking this PR's approach.
| @@ -53,11 +54,16 @@ class CartesianRDD[T: ClassTag, U: ClassTag]( | |||
| extends RDD[(T, U)](sc, Nil) | |||
| with Serializable { | |||
|
|
|||
| val numPartitionsInRdd1 = rdd1.partitions.length | |||
There was a problem hiding this comment.
This change means that rdd1.partitions will be computed possibly more eagerly than it would have been computed before, which is potentially an internal behavior change w.r.t. the old behavior. In principle this should not matter, but if we ever wanted to consider backport of this change then I might weakly incline towards keeping the existing call pattern just for the sake of minimizing the scope of changes.
There was a problem hiding this comment.
@JoshRosen Thank you for your thoughtful advice, I will fix it.
| val numPartitionsInRdd1 = rdd1.partitions.length | ||
| val partitionNum: Long = numPartitionsInRdd1.toLong * numPartitionsInRdd2.toLong | ||
| if (partitionNum > Int.MaxValue) { | ||
| throw SparkCoreErrors.cartesianPartitionNumOverflow(numPartitionsInRdd1, numPartitionsInRdd2) |
There was a problem hiding this comment.
I prefer to reuse COLLECTION_SIZE_LIMIT_EXCEEDED
There was a problem hiding this comment.
@yaooqinn Thank you for your suggestion, let me change it.
@yaooqinn This method is defined in |
|
Please update the PR description @wayneguow |
wayneguow
changed the title
cartesian operation of RDDsCartesianRDD.getPartitions
Both title & description updated~ |
|
Thank you all. @JoshRosen @LuciferYang @yaooqinn |
What changes were proposed in this pull request?
This pr aims to optimize the error message by doing a length check and throw
COLLECTION_SIZE_LIMIT_EXCEEDED.INITIALIZEerror inCartesianRDD.getPartitions.Why are the changes needed?
Optimize the error prompts for Spark users.
Does this PR introduce any user-facing change?
Yes, this PR changes user-facing error class and message.
How was this patch tested?
Add a new test case in
RDDSuite.Was this patch authored or co-authored using generative AI tooling?
No.