manuzhang commented on a change in pull request #29797:
URL: https://github.com/apache/spark/pull/29797#discussion_r491281977
##########
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:
@viirya
This solution is not ideal till we find a way not to apply local shuffle
reader if the partitioning doesn't match that of dynamic partition. It's not
ideal either that any AQE config is global.
With `coalesceShufflePartitionsEnabled` and `localShuffleReaderEnabled`, the
output partitioning of shuffle is uncertain which arguably contradicts the
purpose of `RepartitionByExpression`.
> If the user needs to coalesce shuffle partition in the query, but also
needs dynamic partition overwrite?
Another option is to introduce a new config specific for repartition, e.g.
`spark.sql.adaptive.repartition.canChangeNumPartitions`
----------------------------------------------------------------
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
