kgyrtkirk commented on code in PR #17894:
URL: https://github.com/apache/druid/pull/17894#discussion_r2048673944


##########
sql/src/test/java/org/apache/druid/sql/calcite/util/SqlTestFramework.java:
##########
@@ -852,11 +862,16 @@ public PlannerFixture(
           new DruidHookDispatcher()
       );
       componentSupplier.finalizePlanner(this);
-      this.statementFactory = QueryFrameworkUtils.createSqlStatementFactory(
+      final SqlToolbox toolbox = new SqlToolbox(
           framework.engine,
           plannerFactory,
-          authConfig
+          new ServiceEmitter("dummy", "dummy", new NoopEmitter()),
+          new NoopRequestLogger(),
+          QueryStackTests.DEFAULT_NOOP_SCHEDULER,
+          new DefaultQueryConfig(ImmutableMap.of()),
+          new SqlLifecycleManager()

Review Comment:
   its better to use the injector to obtain the right instances ;  it should 
already have almost all of this...I've seen issue from like a random empty 
query config was picked up somewhere and made things go south...



##########
sql/src/test/java/org/apache/druid/sql/calcite/util/SqlTestFramework.java:
##########
@@ -852,11 +862,16 @@ public PlannerFixture(
           new DruidHookDispatcher()
       );
       componentSupplier.finalizePlanner(this);
-      this.statementFactory = QueryFrameworkUtils.createSqlStatementFactory(
+      final SqlToolbox toolbox = new SqlToolbox(
           framework.engine,
           plannerFactory,
-          authConfig
+          new ServiceEmitter("dummy", "dummy", new NoopEmitter()),
+          new NoopRequestLogger(),
+          QueryStackTests.DEFAULT_NOOP_SCHEDULER,
+          new DefaultQueryConfig(ImmutableMap.of()),
+          new SqlLifecycleManager()
       );
+      this.statementFactory = new TestMultiStatementFactory(toolbox, 
framework.engine, plannerFactory);

Review Comment:
   this seems like a hack to support this in `Calcite*Test` classes
   I wonder if it would be possible to find a different way to somehow instruct 
the production classes to enable the processing of these things?
   
   maybe there could be some `PlannerConfig` knob which could control it in 
which environments its available? 



-- 
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]

Reply via email to