wypoon commented on a change in pull request #2512:
URL: https://github.com/apache/iceberg/pull/2512#discussion_r657252531



##########
File path: 
spark3-extensions/src/main/scala/org/apache/spark/sql/catalyst/utils/DistributionAndOrderingUtils.scala
##########
@@ -80,7 +94,11 @@ object DistributionAndOrderingUtils {
       // the conversion to catalyst expressions above produces SortOrder 
expressions
       // for OrderedDistribution and generic expressions for 
ClusteredDistribution
       // this allows RepartitionByExpression to pick either range or hash 
partitioning
-      RepartitionByExpression(distribution, query, numShufflePartitions)
+      if (Spark3VersionUtil.isSpark30) {
+        repartitionByExpressionCtor.newInstance(distribution.toSeq, query, new 
Integer(numShufflePartitions))
+      } else {
+        repartitionByExpressionCtor.newInstance(distribution.toSeq, query, 
Some(numShufflePartitions))

Review comment:
       Indeed, SPARK-32056, introduced in Spark 3.1, is what changed the 
signature so that the last parameter is an `Option[Int]` instead of an `Int`. 
If you look at the code change for SPARK-32056, if the `Option[Int]` is `None`, 
then the number of partitions is obtained from the conf:
   ```
     val numPartitions = 
optNumPartitions.getOrElse(SQLConf.get.numShufflePartitions)
   ```
   And here, in `DistributionAndOrderingUtils.prepareQuery`, 
`numShufflePartitions` is indeed `conf.numShufflePartitions` from the `SQLConf` 
that is passed in.
   So there is no reason to change the code here.




-- 
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.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to