imply-cheddar commented on code in PR #12867:
URL: https://github.com/apache/druid/pull/12867#discussion_r949482992
##########
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:
Then the answer isn't "I don't have a default implementation because it's
okay to add one" it's "I don't have one because there isn't a good
implementation for them and anybody who implements this interface really does
need to think about what the correct implementation is". In which case, that's
the reason to add a new method, so great :).
Anyway, sorry to be pedantic, but for anything that impacts compatibility,
it's important to show the work that we've thought about the plight of the
developer who is updating their cluster.
--
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]
