szehon-ho commented on code in PR #54492:
URL: https://github.com/apache/spark/pull/54492#discussion_r2875382639


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala:
##########
@@ -370,7 +370,14 @@ abstract class Optimizer(catalogManager: CatalogManager)
       s.withNewPlan(removeTopLevelSort(newPlan))
     }
 
-    def apply(plan: LogicalPlan): LogicalPlan = 
plan.transformAllExpressionsWithPruning(
+    // optimizes subquery expressions, ignoring row-level operation conditions
+    def apply(plan: LogicalPlan): LogicalPlan = plan match {
+      case wd: WriteDelta => wd.copy(query = optimize(wd.query))
+      case rd: ReplaceData => rd.copy(query = optimize(rd.query))
+      case _ => optimize(plan)

Review Comment:
   i think, ReplaceData/WriteDelta can be non-root as well and hence the 
condition still get optimized?



##########
sql/core/src/test/scala/org/apache/spark/sql/connector/RowLevelOperationSuiteBase.scala:
##########
@@ -152,6 +153,22 @@ abstract class RowLevelOperationSuiteBase
     stripAQEPlan(executedPlan)
   }
 
+  // executes an operation and extracts conditions from ReplaceData or 
WriteDelta
+  protected def executeAndKeepConditions(func: => Unit): (Expression, 
Option[Expression]) = {
+    val qes = withQueryExecutionsCaptured(spark)(func)
+    qes.head.optimizedPlan.collectFirst {

Review Comment:
   cosmetic, but maybe this is better (if qes has more than one SparkExecution)
   
   qes.flatMap { qe =>
         qe.optimizedPlan.collectFirst {



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/RewriteMergeIntoTable.scala:
##########
@@ -237,19 +237,21 @@ object RewriteMergeIntoTable extends 
RewriteRowLevelCommand with PredicateHelper
   }
 
   // converts a MERGE condition into an EXISTS subquery for runtime filtering
-  private def toGroupFilterCondition(
+  private def buildGroupFilterCondition(

Review Comment:
   opt: would it be cleaner if the does not call this method if 
groupFilterEnabled == false?  Else have to read this method to see when it 
return None



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to