From 6321beb86d254e5a29c0ede5059621316640636c Mon Sep 17 00:00:00 2001
From: starocean999 <12095047@qq.com>
Date: Wed, 9 Oct 2024 11:39:31 +0800
Subject: [PATCH 1/3] [fix](nereids)adjust agg function nullability in
 PhysicalHashAggregate

---
 .../plans/physical/PhysicalHashAggregate.java | 22 +++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalHashAggregate.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalHashAggregate.java
index c8187727da47f2..dcd531220b414e 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalHashAggregate.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalHashAggregate.java
@@ -27,12 +27,14 @@
 import org.apache.doris.nereids.trees.expressions.NamedExpression;
 import org.apache.doris.nereids.trees.expressions.Slot;
 import org.apache.doris.nereids.trees.expressions.functions.agg.AggregateParam;
+import org.apache.doris.nereids.trees.expressions.functions.agg.NullableAggregateFunction;
 import org.apache.doris.nereids.trees.plans.AggMode;
 import org.apache.doris.nereids.trees.plans.AggPhase;
 import org.apache.doris.nereids.trees.plans.Plan;
 import org.apache.doris.nereids.trees.plans.PlanType;
 import org.apache.doris.nereids.trees.plans.algebra.Aggregate;
 import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor;
+import org.apache.doris.nereids.util.ExpressionUtils;
 import org.apache.doris.nereids.util.MutableState;
 import org.apache.doris.nereids.util.Utils;
 import org.apache.doris.statistics.Statistics;
@@ -93,8 +95,9 @@ public PhysicalHashAggregate(List<Expression> groupByExpressions, List<NamedExpr
         super(PlanType.PHYSICAL_HASH_AGGREGATE, groupExpression, logicalProperties, child);
         this.groupByExpressions = ImmutableList.copyOf(
                 Objects.requireNonNull(groupByExpressions, "groupByExpressions cannot be null"));
-        this.outputExpressions = ImmutableList.copyOf(
-                Objects.requireNonNull(outputExpressions, "outputExpressions cannot be null"));
+        this.outputExpressions = adjustNullableForOutputs(
+                Objects.requireNonNull(outputExpressions, "outputExpressions cannot be null"),
+                groupByExpressions.isEmpty());
         this.partitionExpressions = Objects.requireNonNull(
                 partitionExpressions, "partitionExpressions cannot be null");
         this.aggregateParam = Objects.requireNonNull(aggregateParam, "aggregate param cannot be null");
@@ -120,8 +123,9 @@ public PhysicalHashAggregate(List<Expression> groupByExpressions, List<NamedExpr
                 child);
         this.groupByExpressions = ImmutableList.copyOf(
                 Objects.requireNonNull(groupByExpressions, "groupByExpressions cannot be null"));
-        this.outputExpressions = ImmutableList.copyOf(
-                Objects.requireNonNull(outputExpressions, "outputExpressions cannot be null"));
+        this.outputExpressions = adjustNullableForOutputs(
+                Objects.requireNonNull(outputExpressions, "outputExpressions cannot be null"),
+                groupByExpressions.isEmpty());
         this.partitionExpressions = Objects.requireNonNull(
                 partitionExpressions, "partitionExpressions cannot be null");
         this.aggregateParam = Objects.requireNonNull(aggregateParam, "aggregate param cannot be null");
@@ -330,4 +334,14 @@ public PhysicalHashAggregate<CHILD_TYPE> setTopnPushInfo(TopnPushInfo topnPushIn
         setMutableState(MutableState.KEY_PUSH_TOPN_TO_AGG, topnPushInfo);
         return this;
     }
+
+    private List<NamedExpression> adjustNullableForOutputs(List<NamedExpression> outputs, boolean alwaysNullable) {
+        return ExpressionUtils.rewriteDownShortCircuit(outputs, output -> {
+            if (output instanceof NullableAggregateFunction) {
+                return ((NullableAggregateFunction) output).withAlwaysNullable(alwaysNullable);
+            } else {
+                return output;
+            }
+        });
+    }
 }

From 508b0b2846ce2177f8a2538b29168e22a729226c Mon Sep 17 00:00:00 2001
From: starocean999 <12095047@qq.com>
Date: Wed, 9 Oct 2024 15:43:02 +0800
Subject: [PATCH 2/3] fix case

---
 .../nereids/trees/plans/physical/PhysicalHashAggregate.java    | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalHashAggregate.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalHashAggregate.java
index dcd531220b414e..3212cb170f052c 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalHashAggregate.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalHashAggregate.java
@@ -337,7 +337,8 @@ public PhysicalHashAggregate<CHILD_TYPE> setTopnPushInfo(TopnPushInfo topnPushIn
 
     private List<NamedExpression> adjustNullableForOutputs(List<NamedExpression> outputs, boolean alwaysNullable) {
         return ExpressionUtils.rewriteDownShortCircuit(outputs, output -> {
-            if (output instanceof NullableAggregateFunction) {
+            if (output instanceof NullableAggregateFunction
+                    && ((NullableAggregateFunction) output).isAlwaysNullable() != alwaysNullable) {
                 return ((NullableAggregateFunction) output).withAlwaysNullable(alwaysNullable);
             } else {
                 return output;

From 0575b9d90186d503e56862c4862df520d0326c8a Mon Sep 17 00:00:00 2001
From: starocean999 <12095047@qq.com>
Date: Wed, 9 Oct 2024 17:39:08 +0800
Subject: [PATCH 3/3] fix fe ut

---
 .../doris/nereids/rules/rewrite/AggregateStrategiesTest.java    | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/AggregateStrategiesTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/AggregateStrategiesTest.java
index 34c16309181466..c4a5e53d2ea144 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/AggregateStrategiesTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/AggregateStrategiesTest.java
@@ -138,7 +138,7 @@ public void globalAggregate() {
         Plan root = new LogicalAggregate<>(groupExpressionList, outputExpressionList,
                 true, Optional.empty(), rStudent);
 
-        Sum localOutput0 = new Sum(rStudent.getOutput().get(0).toSlot());
+        Sum localOutput0 = new Sum(false, true, rStudent.getOutput().get(0).toSlot());
 
         PlanChecker.from(MemoTestUtils.createConnectContext(), root)
                 .applyImplementation(twoPhaseAggregateWithoutDistinct())