[GitHub] [spark] ekoifman commented on pull request #30829: [SPARK-33832][SQL] Add an option in AQE to mitigate skew even if it c…

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

ekoifman commented on pull request #30829:
URL: https://github.com/apache/spark/pull/30829#issuecomment-771170612


   The main loop in `AdaptiveSparkPlanExec.getFinalPhysicalPlan()` has this check
   ```
           if (newCost < origCost ||
               (newCost == origCost && currentPhysicalPlan != newPhysicalPlan)) {
   ```
   Why does it check for new shuffles?  Under what circumstances can `reOptimize(logicalPlan)` add a new shuffle (w/o changes proposed in this PR)? 
   
   If we move `OptimizeSkewedJoin` to `queryStagePreparationRules` and it adds a new shuffle, there is no good way that I see to know which new shuffles are "good" and which are "bad".
   


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