morrySnow commented on code in PR #29463:
URL: https://github.com/apache/doris/pull/29463#discussion_r1444136263


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeAggregate.java:
##########
@@ -97,141 +100,165 @@
  * </pre>
  * More example could get from UT {NormalizeAggregateTest}
  */
-public class NormalizeAggregate extends OneRewriteRuleFactory implements 
NormalizeToSlot {
+public class NormalizeAggregate implements RewriteRuleFactory, NormalizeToSlot 
{
     @Override
-    public Rule build() {
-        return 
logicalAggregate().whenNot(LogicalAggregate::isNormalized).then(aggregate -> {
-            // The LogicalAggregate node may contain window agg functions and 
usual agg functions
-            // we call window agg functions as window-agg and usual agg 
functions as trival-agg for short
-            // This rule simplify LogicalAggregate node by:
-            // 1. Push down some exprs from old LogicalAggregate node to a new 
child LogicalProject Node,
-            // 2. create a new LogicalAggregate with normalized group by exprs 
and trival-aggs
-            // 3. Pull up normalized old LogicalAggregate's output exprs to a 
new parent LogicalProject Node
-            // Push down exprs:
-            // 1. all group by exprs
-            // 2. child contains subquery expr in trival-agg
-            // 3. child contains window expr in trival-agg
-            // 4. all input slots of trival-agg
-            // 5. expr(including subquery) in distinct trival-agg
-            // Normalize LogicalAggregate's output.
-            // 1. normalize group by exprs by outputs of bottom LogicalProject
-            // 2. normalize trival-aggs by outputs of bottom LogicalProject
-            // 3. build normalized agg outputs
-            // Pull up exprs:
-            // normalize all output exprs in old LogicalAggregate to build a 
parent project node, typically includes:
-            // 1. simple slots
-            // 2. aliases
-            //    a. alias with no aggs child
-            //    b. alias with trival-agg child
-            //    c. alias with window-agg
+    public List<Rule> buildRules() {
+        return ImmutableList.of(
+                logicalHaving(logicalAggregate()).whenNot(having -> 
having.child().isNormalized())
+                        .then(having -> normalizeAgg(having.child(), having))
+                        .toRule(RuleType.NORMALIZE_AGGREGATE),
+                logicalHaving()
+                        .then(having -> new 
LogicalFilter<>(having.getConjuncts(), having.child()))
+                        .toRule(RuleType.NORMALIZE_AGGREGATE),
+                logicalAggregate().whenNot(LogicalAggregate::isNormalized)
+                        .then(aggregate -> normalizeAgg(aggregate, null))
+                        .toRule(RuleType.NORMALIZE_AGGREGATE));
+    }
 
-            // Push down exprs:
-            // collect group by exprs
-            Set<Expression> groupingByExprs =
-                    ImmutableSet.copyOf(aggregate.getGroupByExpressions());
+    private LogicalPlan normalizeAgg(LogicalAggregate<Plan> aggregate, 
LogicalHaving having) {

Review Comment:
   use Optional<LogicalHaving<?>>



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeAggregate.java:
##########
@@ -97,141 +100,165 @@
  * </pre>
  * More example could get from UT {NormalizeAggregateTest}
  */
-public class NormalizeAggregate extends OneRewriteRuleFactory implements 
NormalizeToSlot {
+public class NormalizeAggregate implements RewriteRuleFactory, NormalizeToSlot 
{
     @Override
-    public Rule build() {
-        return 
logicalAggregate().whenNot(LogicalAggregate::isNormalized).then(aggregate -> {
-            // The LogicalAggregate node may contain window agg functions and 
usual agg functions
-            // we call window agg functions as window-agg and usual agg 
functions as trival-agg for short
-            // This rule simplify LogicalAggregate node by:
-            // 1. Push down some exprs from old LogicalAggregate node to a new 
child LogicalProject Node,
-            // 2. create a new LogicalAggregate with normalized group by exprs 
and trival-aggs
-            // 3. Pull up normalized old LogicalAggregate's output exprs to a 
new parent LogicalProject Node
-            // Push down exprs:
-            // 1. all group by exprs
-            // 2. child contains subquery expr in trival-agg
-            // 3. child contains window expr in trival-agg
-            // 4. all input slots of trival-agg
-            // 5. expr(including subquery) in distinct trival-agg
-            // Normalize LogicalAggregate's output.
-            // 1. normalize group by exprs by outputs of bottom LogicalProject
-            // 2. normalize trival-aggs by outputs of bottom LogicalProject
-            // 3. build normalized agg outputs
-            // Pull up exprs:
-            // normalize all output exprs in old LogicalAggregate to build a 
parent project node, typically includes:
-            // 1. simple slots
-            // 2. aliases
-            //    a. alias with no aggs child
-            //    b. alias with trival-agg child
-            //    c. alias with window-agg
+    public List<Rule> buildRules() {
+        return ImmutableList.of(
+                logicalHaving(logicalAggregate()).whenNot(having -> 
having.child().isNormalized())

Review Comment:
   ```suggestion
                   
logicalHaving(logicalAggregate().whenNot(LogicalAggregate::isNormalized))
   ```



-- 
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]

Reply via email to