This is an automated email from the ASF dual-hosted git repository.

wenchen pushed a commit to branch branch-3.1
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-3.1 by this push:
     new 2bdc4f5  [SPARK-36020][SQL][FOLLOWUP] RemoveRedundantProjects should 
retain the LOGICAL_PLAN_TAG tag
2bdc4f5 is described below

commit 2bdc4f5486988d8c8ca3142ce53eb02259ab0308
Author: Wenchen Fan <wenc...@databricks.com>
AuthorDate: Wed Jul 21 14:03:06 2021 +0800

    [SPARK-36020][SQL][FOLLOWUP] RemoveRedundantProjects should retain the 
LOGICAL_PLAN_TAG tag
    
    ### What changes were proposed in this pull request?
    
    This is a followup of https://github.com/apache/spark/pull/33222 .
    
    https://github.com/apache/spark/pull/33222 made a mistake that, 
`RemoveRedundantProjects` may lose the `LOGICAL_PLAN_TAG` tag, even though the 
logical plan link is retained. This was actually caught by the test 
`LogicalPlanTagInSparkPlanSuite`, but was not being taken care of.
    
    There is no problem so far, but losing information can always lead to 
potential bugs.
    
    ### Why are the changes needed?
    
    fix a mistake
    
    ### Does this PR introduce _any_ user-facing change?
    
    no
    
    ### How was this patch tested?
    
    existing test
    
    Closes #33442 from cloud-fan/minor.
    
    Authored-by: Wenchen Fan <wenc...@databricks.com>
    Signed-off-by: Wenchen Fan <wenc...@databricks.com>
    (cherry picked from commit 94aece4325af8c07160f124997d51b79a4abd242)
    Signed-off-by: Wenchen Fan <wenc...@databricks.com>
---
 .../org/apache/spark/sql/execution/RemoveRedundantProjects.scala    | 6 +++++-
 .../apache/spark/sql/execution/LogicalPlanTagInSparkPlanSuite.scala | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git 
a/sql/core/src/main/scala/org/apache/spark/sql/execution/RemoveRedundantProjects.scala
 
b/sql/core/src/main/scala/org/apache/spark/sql/execution/RemoveRedundantProjects.scala
index eeeb868..8f4ce0f 100644
--- 
a/sql/core/src/main/scala/org/apache/spark/sql/execution/RemoveRedundantProjects.scala
+++ 
b/sql/core/src/main/scala/org/apache/spark/sql/execution/RemoveRedundantProjects.scala
@@ -49,7 +49,11 @@ object RemoveRedundantProjects extends Rule[SparkPlan] {
     plan match {
       case p @ ProjectExec(_, child) =>
         if (isRedundant(p, child, requireOrdering) && canRemove(p, child)) {
-          removeProject(child, requireOrdering)
+          val newPlan = removeProject(child, requireOrdering)
+          // The `newPlan` should retain the logical plan link already. We 
call `setLogicalLink`
+          // here to make sure the `newPlan` sets the `LOGICAL_PLAN_TAG` tag.
+          newPlan.setLogicalLink(child.logicalLink.get)
+          newPlan
         } else {
           p.mapChildren(removeProject(_, false))
         }
diff --git 
a/sql/core/src/test/scala/org/apache/spark/sql/execution/LogicalPlanTagInSparkPlanSuite.scala
 
b/sql/core/src/test/scala/org/apache/spark/sql/execution/LogicalPlanTagInSparkPlanSuite.scala
index dbdb066..5bcec9b 100644
--- 
a/sql/core/src/test/scala/org/apache/spark/sql/execution/LogicalPlanTagInSparkPlanSuite.scala
+++ 
b/sql/core/src/test/scala/org/apache/spark/sql/execution/LogicalPlanTagInSparkPlanSuite.scala
@@ -131,7 +131,7 @@ class LogicalPlanTagInSparkPlanSuite extends 
TPCDSQuerySuite with DisableAdaptiv
   }
 
   private def getLogicalPlan(node: SparkPlan): LogicalPlan = {
-    node.logicalLink.getOrElse {
+    node.getTagValue(SparkPlan.LOGICAL_PLAN_TAG).getOrElse {
       fail(node.getClass.getSimpleName + " does not have a logical plan link")
     }
   }

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org
For additional commands, e-mail: commits-h...@spark.apache.org

Reply via email to