[GitHub] [spark] LuciferYang commented on a diff in pull request #43010: [SPARK-45242][SQL] Use DataFrame ID to semantically validate CollectMetrics

["LuciferYang (via GitHub)" <gi...@apache.org>]

LuciferYang commented on code in PR #43010:
URL: https://github.com/apache/spark/pull/43010#discussion_r1338737506


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/basicLogicalOperators.scala:
##########
@@ -1969,7 +1969,8 @@ trait SupportsSubquery extends LogicalPlan
 case class CollectMetrics(
     name: String,
     metrics: Seq[NamedExpression],
-    child: LogicalPlan)
+    child: LogicalPlan,
+    dataframeId: Long)

Review Comment:
   @amaliujia  when I execute the following command:
   
   ```
   build/mvn clean install -pl connector/connect/server -am -DskipTests
   mvn test -pl connector/connect/server 
   ```
   The following test failures will occur:
   
   ```
   - Test observe *** FAILED ***
     == FAIL: Plans do not match ===
     !CollectMetrics my_metric, [min(id#0) AS min_val#0, max(id#0) AS max_val#0, sum(id#0) AS sum(id)#0L], 0   CollectMetrics my_metric, [min(id#0) AS min_val#0, max(id#0) AS max_val#0, sum(id#0) AS sum(id)#0L], 53
      +- LocalRelation <empty>, [id#0, name#0]                                                                 +- LocalRelation <empty>, [id#0, name#0] (PlanTest.scala:179) 
   ```
   It seems that the failure is due to the differences in `dataframeId` when comparing plans.
   
   For this, I've created SPARK-45357
   



-- 
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: reviews-unsubscribe@spark.apache.org

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