[GitHub] [iceberg] wypoon commented on a change in pull request #2954: [Spark] Support building against both Spark 3.0 and Spark 3.1: follow-up

[GitBox <gi...@apache.org>]

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



##########
File path: spark3-extensions/src/main/scala/org/apache/spark/sql/catalyst/optimizer/RewriteDelete.scala
##########
@@ -95,7 +96,12 @@ case class RewriteDelete(spark: SparkSession) extends Rule[LogicalPlan] with Rew
       case _ =>
         // apply hash partitioning by file if the distribution mode is hash or range
         val numShufflePartitions = conf.numShufflePartitions
-        RepartitionByExpression(Seq(fileNameCol), remainingRowsPlan, numShufflePartitions)
+        val rbeCtor = DistributionAndOrderingUtils.repartitionByExpressionCtor
+        if (Spark3VersionUtil.isSpark30) {
+          rbeCtor.newInstance(Seq(fileNameCol), remainingRowsPlan, Integer.valueOf(numShufflePartitions))
+        } else {
+          rbeCtor.newInstance(Seq(fileNameCol), remainingRowsPlan, Some(numShufflePartitions))
+        }

Review comment:
       Currently, there are 3 catalyst classes used in Iceberg whose constructors changed signatures between Spark 3.0 and 3.1:
   `o.a.s.sql.catalyst.expressions.Alias`
   `o.a.s.sql.catalyst.expressions.SortOrder`
   `o.a.s.sql.catalyst.plans.logical.RepartitionByExpression`
   
   The first 2 have `createAlias` and `createSortOrder` methods respectively (in `o.a.s.sql.catalyst.utils.RewriteRowLevelOperationHelper` and `o.a.s.sql.catalyst.utils.DistributionAndOrderingUtils` respectively) for constructing them.
   I added `createRepartitionByExpression` in `o.a.s.sql.catalyst.utils.PlanUtils`.
   
   Since there is an existing util object for catalyst plans, I put the method there. I'm fine with naming it `newRepartitionByExpression` but I used `createX` since I was using that naming scheme before (for the expressions). I'm also open to moving the methods for the expressions into one util object if you prefer, but that could also be done later.
   
   




-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org