This is an automated email from the ASF dual-hosted git repository.
morrySnow 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 d1cefa974a7 [fix](SVGuard) Preserve session guards during common
expression extraction (#67717)
d1cefa974a7 is described below
commit d1cefa974a76e14d4d6dcaaa42c53cf53d5f34d5
Author: feiniaofeiafei <[email protected]>
AuthorDate: Fri Sep 11 17:05:38 2026 +0800
[fix](SVGuard) Preserve session guards during common expression extraction
(#67717)
### What problem does this PR solve?
Related PR: #58031
Problem Summary: Repeated guarded and unguarded expressions in a
projection can produce aliases that reference outputs defined in the
same projection layer. Keep a session guard and its wrapped root
together during replacement, and expose newly created aliases only after
the entire layer is rewritten. Add unit coverage for guard boundaries
and layer inputs, and a regression case for alias functions queried with
different session settings.
---
.../processor/post/CommonSubExpressionOpt.java | 19 +++-
.../postprocess/CommonSubExpressionTest.java | 105 +++++++++++++++++++++
.../data/nereids_p0/test_cse_session_var_guard.out | 64 +++++++++++++
.../nereids_p0/test_cse_session_var_guard.groovy | 77 +++++++++++++++
4 files changed, 264 insertions(+), 1 deletion(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/CommonSubExpressionOpt.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/CommonSubExpressionOpt.java
index 0d9e3abc25a..f5999111de1 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/CommonSubExpressionOpt.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/CommonSubExpressionOpt.java
@@ -21,6 +21,7 @@ import org.apache.doris.nereids.CascadesContext;
import org.apache.doris.nereids.trees.expressions.Alias;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.SessionVarGuardExpr;
import org.apache.doris.nereids.trees.expressions.Slot;
import
org.apache.doris.nereids.trees.expressions.visitor.DefaultExpressionRewriter;
import org.apache.doris.nereids.trees.plans.Plan;
@@ -76,12 +77,16 @@ public class CommonSubExpressionOpt extends
PlanPostProcessor {
layer.addAll(inputSlots);
Set<Expression> exprsInDepth = CommonSubExpressionCollector
.getExpressionsFromDepthMap(i,
collector.commonExprByDepth);
+ Map<Expression, Alias> currentLayerAliases = new
LinkedHashMap<>();
exprsInDepth.forEach(expr -> {
+ // Only reference aliases produced by earlier layers.
Expression rewritten =
expr.accept(ExpressionReplacer.INSTANCE, aliasMap);
// if rewritten is already alias, use it directly, because
in materialized view rewriting
// Should keep out slot immutably after rewritten
successfully
- aliasMap.put(expr, rewritten instanceof Alias ? (Alias)
rewritten : new Alias(rewritten));
+ currentLayerAliases.put(expr,
+ rewritten instanceof Alias ? (Alias) rewritten :
new Alias(rewritten));
});
+ aliasMap.putAll(currentLayerAliases);
for (Alias alias : aliasMap.values()) {
if (previousAlias.contains(alias)) {
layer.add(alias.toSlot());
@@ -124,5 +129,17 @@ public class CommonSubExpressionOpt extends
PlanPostProcessor {
}
return super.visit(expr, replaceMap);
}
+
+ @Override
+ public Expression visitSessionVarGuardExpr(SessionVarGuardExpr expr,
+ Map<? extends Expression, ? extends Alias> replaceMap) {
+ if (replaceMap.containsKey(expr)) {
+ return replaceMap.get(expr).toSlot();
+ }
+ // Match the collector: the guard and its wrapped root form one
CSE unit.
+ // Replacing the wrapped root would lose its session variable
protection.
+ Expression child = rewriteChildren(this, expr.child(), replaceMap);
+ return child == expr.child() ? expr : expr.withChildren(child);
+ }
}
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/postprocess/CommonSubExpressionTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/postprocess/CommonSubExpressionTest.java
index 1d5ac1dc24a..25c30a9b7ad 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/nereids/postprocess/CommonSubExpressionTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/postprocess/CommonSubExpressionTest.java
@@ -24,19 +24,27 @@ import org.apache.doris.nereids.trees.expressions.Add;
import org.apache.doris.nereids.trees.expressions.Alias;
import org.apache.doris.nereids.trees.expressions.And;
import org.apache.doris.nereids.trees.expressions.ArrayItemReference;
+import org.apache.doris.nereids.trees.expressions.EqualTo;
import org.apache.doris.nereids.trees.expressions.ExprId;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.SessionVarGuardExpr;
import org.apache.doris.nereids.trees.expressions.Slot;
import org.apache.doris.nereids.trees.expressions.SlotReference;
import org.apache.doris.nereids.trees.expressions.functions.scalar.ArrayMap;
+import org.apache.doris.nereids.trees.expressions.functions.scalar.Coalesce;
+import org.apache.doris.nereids.trees.expressions.functions.scalar.If;
import org.apache.doris.nereids.trees.expressions.functions.scalar.Lambda;
import org.apache.doris.nereids.trees.expressions.literal.Literal;
+import org.apache.doris.nereids.trees.expressions.literal.NullLiteral;
+import org.apache.doris.nereids.trees.expressions.literal.StringLiteral;
import
org.apache.doris.nereids.trees.expressions.visitor.DefaultExpressionRewriter;
import org.apache.doris.nereids.types.ArrayType;
import org.apache.doris.nereids.types.IntegerType;
+import org.apache.doris.nereids.types.VarcharType;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
@@ -107,6 +115,103 @@ public class CommonSubExpressionTest extends
ExpressionRewriteTestHelper {
}
+ @Test
+ public void testGuardDoesNotReuseUnguardedRoot() {
+ Slot platform = new SlotReference("platform",
VarcharType.createVarcharType(65533));
+ Expression coalesce = new Coalesce(platform, new StringLiteral(""));
+ SessionVarGuardExpr guard = new SessionVarGuardExpr(coalesce,
+ ImmutableMap.of("enable_decimal256", "false"));
+ Alias unguarded = new Alias(coalesce, "unguarded");
+
+ // Even an available alias for the root cannot replace the protected
computation.
+ Assertions.assertEquals(guard,
guard.accept(CommonSubExpressionOpt.ExpressionReplacer.INSTANCE,
+ ImmutableMap.of(coalesce, unguarded)));
+
+ SessionVarGuardExpr otherGuard = new SessionVarGuardExpr(coalesce,
+ ImmutableMap.of("enable_decimal256", "true"));
+ Assertions.assertEquals(guard,
guard.accept(CommonSubExpressionOpt.ExpressionReplacer.INSTANCE,
+ ImmutableMap.of(otherGuard, new Alias(otherGuard,
"other_session"))));
+ }
+
+ @Test
+ public void testReuseWholeGuardAndGuardedArgument() {
+ Slot platform = new SlotReference("platform",
VarcharType.createVarcharType(65533));
+ Map<String, String> sessionVars = ImmutableMap.of("enable_decimal256",
"false");
+ SessionVarGuardExpr inner = new SessionVarGuardExpr(
+ new Coalesce(platform, new StringLiteral("")), sessionVars);
+ Alias computed = new Alias(inner, "computed");
+ Map<Expression, Alias> aliases = ImmutableMap.of(inner, computed);
+
+ Assertions.assertEquals(computed.toSlot(),
+
inner.accept(CommonSubExpressionOpt.ExpressionReplacer.INSTANCE, aliases));
+
+ SessionVarGuardExpr outer = new SessionVarGuardExpr(
+ new If(new EqualTo(inner, new StringLiteral("")),
NullLiteral.INSTANCE, inner), sessionVars);
+ Expression expected = new SessionVarGuardExpr(
+ new If(new EqualTo(computed.toSlot(), new StringLiteral("")),
+ NullLiteral.INSTANCE, computed.toSlot()), sessionVars);
+ Assertions.assertEquals(expected,
+
outer.accept(CommonSubExpressionOpt.ExpressionReplacer.INSTANCE, aliases));
+ }
+
+ @Test
+ public void testGuardCseProjectionDependencies() throws Exception {
+ Slot platform = new SlotReference("platform",
VarcharType.createVarcharType(65533));
+ Expression coalesce = new Coalesce(platform, new StringLiteral(""));
+ Map<String, String> sessionVars = ImmutableMap.of("enable_decimal256",
"false");
+ SessionVarGuardExpr guardedCoalesce = new
SessionVarGuardExpr(coalesce, sessionVars);
+ Expression guardedIf = new SessionVarGuardExpr(new If(
+ new EqualTo(guardedCoalesce, new StringLiteral("")),
NullLiteral.INSTANCE, guardedCoalesce),
+ sessionVars);
+ Alias x = new Alias(coalesce, "x");
+ Alias y = new Alias(coalesce, "y");
+ Alias z = new Alias(guardedIf, "z");
+ Method method = CommonSubExpressionOpt.class
+ .getDeclaredMethod("computeMultiLayerProjections", Set.class,
List.class);
+ method.setAccessible(true);
+
+ // Use the expanded alias-function reproducer in both projection
orders.
+ for (List<NamedExpression> projects : ImmutableList.of(
+ ImmutableList.<NamedExpression>of(x, y, z),
ImmutableList.<NamedExpression>of(z, x, y))) {
+ List<List<NamedExpression>> layers = (List<List<NamedExpression>>)
method.invoke(
+ new CommonSubExpressionOpt(), coalesce.getInputSlots(),
projects);
+ Assertions.assertEquals(2, layers.size());
+ Map<Expression, Alias> extracted = new HashMap<>();
+ for (NamedExpression expression : layers.get(0)) {
+ if (expression instanceof Alias) {
+ extracted.put(expression.child(0), (Alias) expression);
+ }
+ }
+ Assertions.assertEquals(2, extracted.size());
+ Assertions.assertTrue(extracted.containsKey(coalesce));
+ Assertions.assertTrue(extracted.containsKey(guardedCoalesce));
+
+ // Check the entire layer before making any of its outputs
available.
+ Set<Slot> inputs = new HashSet<>(coalesce.getInputSlots());
+ for (List<NamedExpression> layer : layers) {
+ Set<Slot> outputs = new HashSet<>();
+ for (NamedExpression expression : layer) {
+
Assertions.assertTrue(inputs.containsAll(expression.getInputSlots()),
+ "Projection references an unavailable input: " +
expression);
+ outputs.add(expression.toSlot());
+ }
+ inputs = outputs;
+ }
+ Slot guardedSlot = extracted.get(guardedCoalesce).toSlot();
+ Expression expectedIf = new SessionVarGuardExpr(new If(
+ new EqualTo(guardedSlot, new StringLiteral("")),
NullLiteral.INSTANCE, guardedSlot), sessionVars);
+ Map<ExprId, Expression> expected = ImmutableMap.of(
+ x.getExprId(), extracted.get(coalesce).toSlot(),
+ y.getExprId(), extracted.get(coalesce).toSlot(),
z.getExprId(), expectedIf);
+ for (int i = 0; i < projects.size(); i++) {
+ NamedExpression output = layers.get(1).get(i);
+ Assertions.assertEquals(projects.get(i).getExprId(),
output.getExprId());
+ Assertions.assertEquals(projects.get(i).getName(),
output.getName());
+ Assertions.assertEquals(expected.get(output.getExprId()),
output.child(0));
+ }
+ }
+ }
+
private void assertExpression(Expression expr, String str) {
Assertions.assertEquals(ExprParser.INSTANCE.parseExpression(str),
expr);
}
diff --git a/regression-test/data/nereids_p0/test_cse_session_var_guard.out
b/regression-test/data/nereids_p0/test_cse_session_var_guard.out
new file mode 100644
index 00000000000..d1faf94beb0
--- /dev/null
+++ b/regression-test/data/nereids_p0/test_cse_session_var_guard.out
@@ -0,0 +1,64 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !without_guard --
+1 abc abc abc
+2 \N
+3 \N
+4 xyz xyz xyz
+5 abc abc abc
+6
+7 中文 中文 中文
+
+-- !with_guard --
+1 abc abc abc
+2 \N
+3 \N
+4 xyz xyz xyz
+5 abc abc abc
+6
+7 中文 中文 中文
+
+-- !guard_first --
+1 abc abc abc
+2 \N
+3 \N
+4 xyz xyz xyz
+5 abc abc abc
+6
+7 中文 中文 中文
+
+-- !repeated_guard --
+1 abc abc abc abc
+2 \N \N
+3 \N \N
+4 xyz xyz xyz xyz
+5 abc abc abc abc
+6
+7 中文 中文 中文 中文
+
+-- !nested_guard --
+1 fallback fallback fallback
+2 \N
+3 fallback fallback fallback
+4 xyz xyz xyz
+5 fallback fallback fallback
+6
+7 中文 中文 中文
+
+-- !multiple_layers --
+1 2 4 6
+2 3 6 9
+3 4 8 12
+4 5 10 15
+5 6 12 18
+6 7 14 21
+7 8 16 24
+
+-- !reverse_session_guard --
+1 abc abc abc
+2 \N
+3 \N
+4 xyz xyz xyz
+5 abc abc abc
+6
+7 中文 中文 中文
+
diff --git
a/regression-test/suites/nereids_p0/test_cse_session_var_guard.groovy
b/regression-test/suites/nereids_p0/test_cse_session_var_guard.groovy
new file mode 100644
index 00000000000..2862155be51
--- /dev/null
+++ b/regression-test/suites/nereids_p0/test_cse_session_var_guard.groovy
@@ -0,0 +1,77 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite("test_cse_session_var_guard") {
+ sql "DROP TABLE IF EXISTS cse_guard_demo"
+ sql """CREATE TABLE cse_guard_demo (id INT, platform VARCHAR(65533))
+ DISTRIBUTED BY HASH(id) BUCKETS 1 PROPERTIES("replication_num" =
"1")"""
+ sql """INSERT INTO cse_guard_demo VALUES
+ (1, 'abc'), (2, ''), (3, NULL), (4, 'xyz'), (5, 'abc'), (6, ' '), (7,
'中文')"""
+ sql "DROP FUNCTION IF EXISTS cse_guard_replace_null(VARCHAR(65533))"
+ sql "SET enable_decimal256 = false"
+ sql """CREATE ALIAS FUNCTION cse_guard_replace_null(VARCHAR(65533))
+ WITH PARAMETER(foo) AS IF(foo = '', NULL, foo)"""
+
+ // Evaluate the original projection without guards as a result baseline.
+ qt_without_guard """SELECT id, coalesce(platform, '') AS x,
coalesce(platform, '') AS y,
+ cse_guard_replace_null(coalesce(platform, '')) AS z
+ FROM cse_guard_demo ORDER BY id"""
+
+ // The function was created with decimal256 disabled. Changing it adds
guards
+ // to the expanded IF and COALESCE expressions, even for VARCHAR arguments.
+ sql "SET enable_decimal256 = true"
+ sql """EXPLAIN SELECT coalesce(platform, '') AS x, coalesce(platform, '')
AS y,
+ cse_guard_replace_null(coalesce(platform, '')) AS z FROM
cse_guard_demo"""
+ qt_with_guard """SELECT id, coalesce(platform, '') AS x,
coalesce(platform, '') AS y,
+ cse_guard_replace_null(coalesce(platform, '')) AS z
+ FROM cse_guard_demo ORDER BY id"""
+
+ // Reverse discovery order: guarded and unguarded roots still cannot depend
+ // on aliases produced in their own projection layer.
+ qt_guard_first """SELECT id, cse_guard_replace_null(coalesce(platform,
'')) AS z,
+ coalesce(platform, '') AS x, coalesce(platform, '') AS y
+ FROM cse_guard_demo ORDER BY id"""
+
+ // Reusing an entire guarded expression must remain supported.
+ qt_repeated_guard """SELECT id,
+ cse_guard_replace_null(coalesce(platform, '')) AS z1,
+ cse_guard_replace_null(coalesce(platform, '')) AS z2,
+ coalesce(platform, '') AS x, coalesce(platform, '') AS y
+ FROM cse_guard_demo ORDER BY id"""
+
+ // Common expressions below the guarded root may still be extracted into
+ // earlier layers without separating a guarded root from its guard.
+ qt_nested_guard """SELECT id,
+ coalesce(nullif(platform, 'abc'), 'fallback') AS x,
+ coalesce(nullif(platform, 'abc'), 'fallback') AS y,
+ cse_guard_replace_null(coalesce(nullif(platform, 'abc'), 'fallback'))
AS z
+ FROM cse_guard_demo ORDER BY id"""
+
+ // Ordinary CSE must retain reuse across multiple projection layers.
+ qt_multiple_layers """SELECT id, id + 1 AS x,
+ (id + 1) * 2 AS y, (id + 1) * 2 + (id + 1) AS z
+ FROM cse_guard_demo ORDER BY id"""
+
+ // Also cover a function created with decimal256 enabled and queried with
it disabled.
+ sql "DROP FUNCTION IF EXISTS cse_guard_replace_null(VARCHAR(65533))"
+ sql """CREATE ALIAS FUNCTION cse_guard_replace_null(VARCHAR(65533))
+ WITH PARAMETER(foo) AS IF(foo = '', NULL, foo)"""
+ sql "SET enable_decimal256 = false"
+ qt_reverse_session_guard """SELECT id, coalesce(platform, '') AS x,
coalesce(platform, '') AS y,
+ cse_guard_replace_null(coalesce(platform, '')) AS z
+ FROM cse_guard_demo ORDER BY id"""
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
