ZiyaZa commented on code in PR #55576:
URL: https://github.com/apache/spark/pull/55576#discussion_r3193557307
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/WriteToDataSourceV2Exec.scala:
##########
@@ -350,9 +350,15 @@ case class ReplaceDataExec(
// One of the metrics couldn't be found, also mark numDeletedRows as
not found.
-1L
}
+
+ // SQLMetric.set call is a no-op if value is -1. Override numDeletedRows
value in summary.
metrics("numDeletedRows").set(numDeletedRows)
+ super.getWriteSummary(query).map {
Review Comment:
I applied what you suggested, except I made the new summary getters return
the subclass type (e.g., `buildDeleteSummary` returns
`Option[DeleteSummaryImpl]`). This way in the override we can do:
`super.getDeleteSummary().map(_.copy(numDeletedRows = numDeletedRows))`.
I believe calling the super here and overwriting `numDeletedRows` is future
proof if we ever add more fields / calculations to the base function. But if
you have strong opinions here, I can also construct `DeleteSummaryImpl` in
place instead of calling super.
--
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]
