rohangarg commented on code in PR #12867:
URL: https://github.com/apache/druid/pull/12867#discussion_r949458759
##########
processing/src/main/java/org/apache/druid/query/GenericQueryMetricsFactory.java:
##########
@@ -52,11 +50,7 @@
/**
* Creates a {@link QueryMetrics} which doesn't have predefined QueryMetrics
subclass. This method is used
* by the router to build a {@link QueryMetrics} for SQL queries. It is
needed since at router, there is no native
- * query linked to a SQL query. By default, it returns null to maintain
backward compatibility.
+ * query linked to a SQL query.
*/
- @Nullable
- default QueryMetrics<Query<?>> makeMetrics()
- {
- return null;
- }
+ QueryMetrics<Query<?>> makeMetrics();
Review Comment:
I'm not sure if the default would be a good thing to have here. having a
default method in the interface which returns `null` would mean that all the
developers who have written custom `GenericQueryMetricsFactory` would need to
implement this method anyways to get router metrics. Also this information to
extend the factory would need communication.
Further since the method return would be a nullable, all users of the method
would need to handle null explicitly in the code from now on.
The good thing that happens with default implementation is that developers
who don't care about router metrics for SQL queries don't need to make any code
changes to their custom implementation of metrics factory.
--
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]
