xiangfu0 commented on code in PR #18554:
URL: https://github.com/apache/pinot/pull/18554#discussion_r3281051373
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java:
##########
@@ -1025,7 +1033,11 @@ public static class PlannerRuleNames {
PlannerRuleNames.AGGREGATE_UNION_AGGREGATE,
PlannerRuleNames.JOIN_TO_ENRICHED_JOIN,
PlannerRuleNames.AGGREGATE_FUNCTION_REWRITE,
- PlannerRuleNames.JOIN_PUSH_TRANSITIVE_PREDICATES
+ PlannerRuleNames.JOIN_PUSH_TRANSITIVE_PREDICATES,
+ // Stock Calcite rule kept opt-in via usePlannerRules — see
SORT_PROJECT_TRANSPOSE javadoc
+ // above for the rationale (firing in BASIC_RULES disrupts
ProjectToSemiJoinRule on
+ // partition-hinted IN(SELECT) queries, breaking colocated broadcast
semi-joins).
+ PlannerRuleNames.SORT_PROJECT_TRANSPOSE
Review Comment:
Keeping `SortProjectTranspose` opt-in only via `DEFAULT_DISABLED_RULES` is
not enough. Brokers that already set `pinot.broker.mse.planner.disabled.rules`
bypass that default set entirely (`MultiStageBrokerRequestHandler` treats the
configured list as a full replacement), so this rule becomes enabled
immediately after upgrade for those clusters. That re-exposes the exact
partition-hinted `IN (SELECT)` semi-join regression the PR description calls
out unless the configured disables are merged with the defaults or the rule is
moved out of `BASIC_RULES`.
--
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]
