This is an automated email from the ASF dual-hosted git repository.
924060929 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 9856352102d [fix](eager-agg) Prevent unsafe concat_ws aggregate
pushdown through outer joins (#68182)
9856352102d is described below
commit 9856352102d05e0c412de6c47417aa3ff6c36351
Author: feiniaofeiafei <[email protected]>
AuthorDate: Mon Sep 21 16:26:59 2026 +0800
[fix](eager-agg) Prevent unsafe concat_ws aggregate pushdown through outer
joins (#68182)
### What problem does this PR solve?
With a non-null separator, `concat_ws(',', NULL)` returns an empty
string. Eager aggregation must preserve the contribution of such values
from unmatched outer-join rows. Without the `NullToNonNullFunction`
marker, aggregates containing `concat_ws` can be pushed below the
nullable side of an outer join and produce incorrect results.
Make `ConcatWs` implement `NullToNonNullFunction` to reuse the existing
pushdown protection. Add coverage in `EagerAggRewriterTest` for both
left and right outer joins, and verify that inner-join pushdown remains
allowed.
### Release note
Fix incorrect eager aggregation results for `concat_ws` on the nullable
side of outer joins.
### Check List (For Author)
- Test
- [x] Unit Test: `bash run-fe-ut.sh --run
org.apache.doris.nereids.rules.rewrite.eageraggregation.EagerAggRewriterTest`
— 31 tests passed.
- Checkstyle 9.3 passed for both changed Java files using the repository
configuration. Standalone Maven validation was blocked by unresolved
internal SNAPSHOT dependencies, so Checkstyle was run directly.
- `git diff --check` passed.
- Behavior changed:
- [x] Yes. Prevent unsafe aggregate pushdown for `concat_ws` on the
nullable side of outer joins.
---
.../expressions/functions/scalar/ConcatWs.java | 3 +-
.../eageraggregation/EagerAggRewriterTest.java | 37 ++++++++++++++++++++++
2 files changed, 39 insertions(+), 1 deletion(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ConcatWs.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ConcatWs.java
index 1a0382e4e1f..f120d7232a9 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ConcatWs.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ConcatWs.java
@@ -19,6 +19,7 @@ package
org.apache.doris.nereids.trees.expressions.functions.scalar;
import org.apache.doris.catalog.FunctionSignature;
import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.NullToNonNullFunction;
import
org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature;
import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
import org.apache.doris.nereids.types.ArrayType;
@@ -35,7 +36,7 @@ import java.util.List;
* ScalarFunction 'concat_ws'. This class is generated by GenerateFunction.
*/
public class ConcatWs extends ScalarFunction
- implements ExplicitlyCastableSignature {
+ implements ExplicitlyCastableSignature, NullToNonNullFunction {
public static final List<FunctionSignature> SIGNATURES = ImmutableList.of(
FunctionSignature.ret(VarcharType.SYSTEM_DEFAULT)
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/eageraggregation/EagerAggRewriterTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/eageraggregation/EagerAggRewriterTest.java
index c44e76c9616..c22cdad81ce 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/eageraggregation/EagerAggRewriterTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/rules/rewrite/eageraggregation/EagerAggRewriterTest.java
@@ -716,6 +716,43 @@ class EagerAggRewriterTest extends TestWithFeService
implements MemoPatternMatch
}
}
+ @Test
+ void testNotPushConcatWsToNullableSideOfOuterJoin() {
+ // concat_ws(',', NULL) returns an empty string, so unmatched rows
contribute to count.
+ // Pushing the aggregate to the nullable side would lose these
contributions.
+ connectContext.getSessionVariable().setEagerAggregationMode(1);
+ connectContext.getSessionVariable().setDisableJoinReorder(true);
+ try {
+ String sql = "select count(concat_ws(',', t1.name)), t2.id2"
+ + " from t1 right join t2 on t1.id1 = t2.id2 group by
t2.id2";
+ PlanChecker.from(connectContext)
+ .analyze(sql)
+ .rewrite()
+ .nonMatch(logicalJoin(logicalAggregate(), any()))
+ .printlnTree();
+
+ sql = "select count(concat_ws(',', t2.name)), t1.id1"
+ + " from t1 left join t2 on t1.id1 = t2.id2 group by
t1.id1";
+ PlanChecker.from(connectContext)
+ .analyze(sql)
+ .rewrite()
+ .nonMatch(logicalJoin(any(), logicalAggregate()))
+ .printlnTree();
+
+ // Inner joins do not introduce null-extended rows, so pushdown
remains safe.
+ sql = "select count(concat_ws(',', t1.name)), t2.id2"
+ + " from t1 join t2 on t1.id1 = t2.id2 group by t2.id2";
+ PlanChecker.from(connectContext)
+ .analyze(sql)
+ .rewrite()
+
.matches(logicalAggregate(logicalProject(logicalJoin(logicalAggregate(),
any()))))
+ .printlnTree();
+ } finally {
+ connectContext.getSessionVariable().setEagerAggregationMode(0);
+ connectContext.getSessionVariable().setDisableJoinReorder(false);
+ }
+ }
+
@Test
void testNotPushNvlToNullableSideOfOuterJoin() {
// count(nvl(col, default)): NVL converts NULL input to default.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
