github-actions[bot] commented on code in PR #64849:
URL: https://github.com/apache/doris/pull/64849#discussion_r3480580327
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java:
##########
@@ -675,7 +675,7 @@ public class Rewriter extends AbstractBatchJobExecutor {
cascadesContext ->
cascadesContext.rewritePlanContainsTypes(LogicalAggregate.class)
||
cascadesContext.rewritePlanContainsTypes(LogicalJoin.class)
||
cascadesContext.rewritePlanContainsTypes(LogicalUnion.class),
- topDown(new EliminateGroupByKey()),
+ custom(RuleType.ELIMINATE_GROUP_BY_KEY,
EliminateGroupByKey::new),
Review Comment:
This changes the session-rule contract for the filter variant. The old
implementation registered `Project(Filter(Aggregate))` as
`ELIMINATE_FILTER_GROUP_BY_KEY`, and that rule name still exists and is still
exposed in disable-rule lists, but this custom job is registered only as
`ELIMINATE_GROUP_BY_KEY`. `CustomRewriteJob` only checks the single registered
`ruleType`, while `EliminateGroupByKey.visitLogicalProject()` still rewrites
the `LogicalFilter` child path. A session or MV path that disables only
`ELIMINATE_FILTER_GROUP_BY_KEY` will now still get this rewrite. Please either
preserve the old disable knob for the filter branch, or remove/redirect the
stale rule id with coverage that proves the intended compatibility behavior.
--
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]
