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