cloud-fan commented on a change in pull request #31476:
URL: https://github.com/apache/spark/pull/31476#discussion_r572298710



##########
File path: 
sql/catalyst/src/main/java/org/apache/spark/sql/connector/read/Scan.java
##########
@@ -102,4 +102,13 @@ default MicroBatchStream toMicroBatchStream(String 
checkpointLocation) {
   default ContinuousStream toContinuousStream(String checkpointLocation) {
     throw new UnsupportedOperationException(description() + ": Continuous scan 
are not supported");
   }
+
+  /**
+   * Returns an array of supported custom metrics with name and description.
+   * By default it returns empty array.
+   */
+  default CustomMetric[] supportedCustomMetrics() {

Review comment:
       I don't see how a mixin trait helps avoid breaking change. Removing the 
trait later is also a breaking change. Agree with @rdblue that an optional 
method is simple and sufficient. Some other features have mixin trait because 
they don't have a reasonable default if made into optional methods. Metrics is 
OK as we can return empty metrics as the default.




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



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

Reply via email to