jihoonson commented on code in PR #12396:
URL: https://github.com/apache/druid/pull/12396#discussion_r853714434
##########
sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java:
##########
@@ -8269,10 +8280,14 @@ public void
testQueryWithSelectProjectAndIdentityProjectDoesNotRename() throws E
cannotVectorize();
requireMergeBuffers(3);
testQuery(
- PLANNER_CONFIG_NO_HLL.withOverrides(ImmutableMap.of(
- PlannerConfig.CTX_KEY_USE_GROUPING_SET_FOR_EXACT_DISTINCT,
- "true"
- )),
+ PLANNER_CONFIG_NO_HLL.withOverrides(
+ new QueryContext(
+ ImmutableMap.of(
+ PlannerConfig.CTX_KEY_USE_GROUPING_SET_FOR_EXACT_DISTINCT,
+ "true"
+ )
+ )
Review Comment:
I'm not sure if that is worth. I think we rather need to refactor and split
`CalciteQueryTest` since this class is too big now.
##########
sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidJoinRule.java:
##########
@@ -74,7 +73,7 @@ private DruidJoinRule(final PlannerContext plannerContext)
operand(DruidRel.class, any())
)
);
- this.enableLeftScanDirect =
QueryContexts.getEnableJoinLeftScanDirect(plannerContext.getQueryContext());
Review Comment:
IMO, now `QueryContexts` should remain just as a holder of predefined
context parameters. It doesn't give any more benefit over `QueryContext`.
`getEnableJoinLeftScanDirect` is just a util method for our convenience and
they all should be moved to `QueryContext`.
--
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]
