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