Github user seancxmao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/23258#discussion_r240024723
  
    --- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/metric/SQLMetricsSuite.scala
 ---
    @@ -182,10 +182,13 @@ class SQLMetricsSuite extends SparkFunSuite with 
SQLMetricsTestUtils with Shared
       }
     
       test("Sort metrics") {
    -    // Assume the execution plan is
    -    // WholeStageCodegen(nodeId = 0, Range(nodeId = 2) -> Sort(nodeId = 1))
    -    val ds = spark.range(10).sort('id)
    -    testSparkPlanMetrics(ds.toDF(), 2, Map.empty)
    +    // Assume the execution plan with node id is
    +    // Sort(nodeId = 0)
    +    //   Exchange(nodeId = 1)
    +    //     LocalTableScan(nodeId = 2)
    +    val df = Seq(1, 3, 2).toDF("id").sort('id)
    +    testSparkPlanMetrics(df, 2, Map.empty)
    --- End diff --
    
    @mgaido91 This case tries to check `Sort` (nodeId=0) metrics, rather than 
`LocalTableScan`. The second parameter (`2`) of `testSparkPlanMetrics(df, 2, 
Map.empty)` means `expectedNumOfJobs` rather than `nodeId`. The third parameter 
`expectedMetrics` will pass `nodeId` together with corresponding expected 
metrics. Because metrics of Sort node (including `sortTime`, `peakMemory`, 
`spillSize`) may change during each execution, unlike metrics like 
`numOutputRows`, we have no way to check these values.


---

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

Reply via email to