[GitHub] [spark] viirya commented on a change in pull request #29797: [SPARK-32932][SQL] Do not use local shuffle reader on RepartitionByExpression when coalescing disabled

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

viirya commented on a change in pull request #29797:
URL: https://github.com/apache/spark/pull/29797#discussion_r490732936



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
##########
@@ -705,7 +705,8 @@ abstract class SparkStrategies extends QueryPlanner[SparkPlan] {
         exchange.ShuffleExchangeExec(
           r.partitioning,
           planLater(r.child),
-          noUserSpecifiedNumPartition = r.optNumPartitions.isEmpty) :: Nil
+          noUserSpecifiedNumPartition = conf.coalesceShufflePartitionsEnabled &&
+            r.optNumPartitions.isEmpty) :: Nil

Review comment:
       Two questions so far.
   
   1. Seems to me `RepartitionByExpression` here is not alwasys for dynamic partition overwrite case. By this change, all `RepartitionByExpression` are affected by `coalesceShufflePartitionsEnabled` config.
   
   2. So the users of dynamic partition overwrite need to set `conf.coalesceShufflePartitionsEnabled` false to avoid that? If the user needs to coalesce shuffle partition in the query, but also needs dynamic partition overwrite?




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org