yashmayya commented on code in PR #15027:
URL: https://github.com/apache/pinot/pull/15027#discussion_r1959174818


##########
pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotRuleUtils.java:
##########
@@ -46,10 +46,12 @@ private PinotRuleUtils() {
   public static final RelBuilderFactory PINOT_REL_FACTORY =
       RelBuilder.proto(Contexts.of(RelFactories.DEFAULT_STRUCT, 
PINOT_REL_CONFIG));
 
-  public static final SqlToRelConverter.Config PINOT_SQL_TO_REL_CONFIG =
-      
SqlToRelConverter.config().withHintStrategyTable(PinotHintStrategyTable.PINOT_HINT_STRATEGY_TABLE)
-          
.withTrimUnusedFields(true).withExpand(true).withInSubQueryThreshold(Integer.MAX_VALUE)
-          .withRelBuilderFactory(PINOT_REL_FACTORY);
+  public static final SqlToRelConverter.Config PINOT_SQL_TO_REL_CONFIG = 
SqlToRelConverter.config()
+      .withHintStrategyTable(PinotHintStrategyTable.PINOT_HINT_STRATEGY_TABLE)
+      .withTrimUnusedFields(true)
+      .withExpand(true)
+      .withInSubQueryThreshold(0)

Review Comment:
   I think Jackie's goal is to always convert `IN` to semi-join and `NOT IN` to 
left-join in the `SqlToRelConverter` and then translate those back to `IN` / 
`NOT IN` via the new rules? If we use the default value of `20`, we'll need to 
also add rules to convert the OR'd expressions back to `IN` / `NOT IN` 
(although they're not particularly problematic if < 20 and we could just let 
them be I suppose).



-- 
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: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to