imply-cheddar commented on code in PR #16624:
URL: https://github.com/apache/druid/pull/16624#discussion_r1695362240
##########
sql/src/main/java/org/apache/druid/sql/calcite/planner/QueryHandler.java:
##########
@@ -563,7 +563,7 @@ protected PlannerResult planWithDruidConvention() throws
ValidationException
.plus(DruidLogicalConvention.instance()),
newRoot
);
- Hook.JAVA_PLAN.run(newRoot);
Review Comment:
I think it would be a bit nicer if this didn't have to rely on statics and
could instead rely on an instance object. For this `QueryHandler` itself, it
looks like these end up constructed via the `DruidPlanner` which is built by
the `PlannerFactory` which is injected via Guice. Could we get the `DruidHook`
(or something equivalent to it) injected into that and pushed down to this
object instead of relying on statics?
--
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]
