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

Reply via email to