Jackie-Jiang commented on code in PR #16238: URL: https://github.com/apache/pinot/pull/16238#discussion_r2176168822
########## pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotQueryRuleSets.java: ########## @@ -98,7 +102,12 @@ private PinotQueryRuleSets() { .instanceWithDescription(PlannerRuleNames.EVALUATE_LITERAL_FILTER), // sort join rules - // TODO: evaluate the SORT_JOIN_TRANSPOSE and SORT_JOIN_COPY rules + // push sort through join for left/right outer join only, disabled by default + SortJoinTransposeRule.Config.DEFAULT + .withDescription(PlannerRuleNames.SORT_JOIN_TRANSPOSE).toRule(), Review Comment: Format is wrong ########## pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java: ########## @@ -181,19 +181,30 @@ public static Map<String, Set<FieldConfig.IndexType>> getSkipIndexes(Map<String, return skipIndexes; } - @Nullable public static Set<String> getSkipPlannerRules(Map<String, String> queryOptions) { - // Example config: skipPlannerRules='FilterIntoJoinRule,FilterAggregateTransposeRule' + // Example config: skipPlannerRules='FilterIntoJoin,FilterAggregateTranspose' String skipIndexesStr = queryOptions.get(QueryOptionKey.SKIP_PLANNER_RULES); if (skipIndexesStr == null) { - return null; + return Collections.emptySet(); Review Comment: (minor) Use `Set.of()`, same for other places -- 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