[GitHub] [spark] c21 commented on a change in pull request #31477: [SPARK-34369][SQL] Track number of pairs processed out of Join

[GitBox <gi...@apache.org>]

c21 commented on a change in pull request #31477:
URL: https://github.com/apache/spark/pull/31477#discussion_r577343739



##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/joins/HashJoin.scala
##########
@@ -48,6 +48,13 @@ trait HashJoin extends BaseJoinExec with CodegenSupport {
     s"$nodeName $joinType ${buildSide} ($opId)".trim
   }
 
+  /**
+   * When overriding metrics be sure to include numOutputRows & numMatchedRows
+   */
+  override lazy val metrics = Map(
+    "numOutputRows" -> SQLMetrics.createMetric(sparkContext, "number of output rows"),
+    "numMatchedRows" -> SQLMetrics.createMetric(sparkContext, "number of matched rows"))

Review comment:
       I feel it's kind of weird that we need to double define `metrics` here in `HashJoin`, and also its subclass `BroadcastHashJoinExec` and `ShuffledHashJoinExec`. It might introduce unexpected bug later. Could we pass the `numMatchedRows` to `HashJoin`, similar to what we did for `numOutputRows` before?




----------------------------------------------------------------
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:
users@infra.apache.org



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