sigmod commented on a change in pull request #32742:
URL: https://github.com/apache/spark/pull/32742#discussion_r644555757
##########
File path:
sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/AdaptiveSparkPlanExec.scala
##########
@@ -603,6 +603,19 @@ case class AdaptiveSparkPlanExec(
(newPlan, optimized)
}
+ /**
+ * Clean up logical plan stats before re-optimize
+ */
+ private def cleanupStats(logicalPlan: LogicalPlan): Unit = {
+ logicalPlan.invalidateStatsCache()
+ // We must invalidate ineffective rules before re-optimize since AQE
Optimizer may introduce
+ // LocalRelation that can affect result.
Review comment:
Why do we need to invalidate ineffective rules? If AQE Optimizer
rewrote a leaf node to `LocalRelation`, the path from the new `LocalRelation`
to the new root are all *new* TreeNode instances, in which ineffective rule
bits are not set.
##########
File path:
sql/core/src/main/scala/org/apache/spark/sql/execution/adaptive/AQEPropagateEmptyRelation.scala
##########
@@ -53,8 +54,8 @@ object AQEPropagateEmptyRelation extends
PropagateEmptyRelationBase {
empty(j)
}
- // TODO we need use transformUpWithPruning instead of transformUp
- def apply(plan: LogicalPlan): LogicalPlan = plan.transformUp {
+ def apply(plan: LogicalPlan): LogicalPlan = plan.transformUpWithPruning(
+ _.containsAnyPattern(LOGICAL_QUERY_STAGE, LOCAL_RELATION,
TRUE_OR_FALSE_LITERAL), ruleId) {
Review comment:
Can AQEPropagateEmptyRelation be invoked multiple times on the same tree
or subtree?
If not, then we don't need "ruleId".
--
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]
