This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new cb33485bfe3 [fix](fe)eliminate noop cast in thrift by overriding
treeToThriftHelper (#67192)
cb33485bfe3 is described below
commit cb33485bfe38557b0a2e1a36814e203403d4a7c5
Author: starocean999 <[email protected]>
AuthorDate: Fri Aug 28 01:31:37 2026 +0800
[fix](fe)eliminate noop cast in thrift by overriding treeToThriftHelper
(#67192)
master is ok, this bug only exists in 4.1 and 4.0
Issue Number: close #xxx
Related PR: https://github.com/apache/doris/pull/39670
Problem Summary:
CastExpr's treeToThriftHelper function eliminate noop cast, but there is
another override version shoud be handled.
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
---
.../java/org/apache/doris/analysis/CastExpr.java | 6 +-
.../CastExprThriftSimplificationTest.java | 119 +++++++++++++++++++++
2 files changed, 122 insertions(+), 3 deletions(-)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/CastExpr.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/CastExpr.java
index 75b20cc45e1..8c15fb43cc5 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/CastExpr.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/CastExpr.java
@@ -114,12 +114,12 @@ public class CastExpr extends Expr {
}
@Override
- protected void treeToThriftHelper(TExpr container) {
+ protected void treeToThriftHelper(TExpr container, ExprVisitor visitor) {
if (noOp) {
- getChild(0).treeToThriftHelper(container);
+ getChild(0).treeToThriftHelper(container, visitor);
return;
}
- super.treeToThriftHelper(container);
+ super.treeToThriftHelper(container, visitor);
}
@Override
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/nereids/glue/translator/CastExprThriftSimplificationTest.java
b/fe/fe-core/src/test/java/org/apache/doris/nereids/glue/translator/CastExprThriftSimplificationTest.java
new file mode 100644
index 00000000000..3c3fc941fdb
--- /dev/null
+++
b/fe/fe-core/src/test/java/org/apache/doris/nereids/glue/translator/CastExprThriftSimplificationTest.java
@@ -0,0 +1,119 @@
+// 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.
+
+package org.apache.doris.nereids.glue.translator;
+
+import org.apache.doris.analysis.Expr;
+import org.apache.doris.nereids.NereidsPlanner;
+import org.apache.doris.nereids.StatementContext;
+import org.apache.doris.nereids.parser.NereidsParser;
+import org.apache.doris.nereids.properties.PhysicalProperties;
+import org.apache.doris.nereids.trees.plans.physical.PhysicalPlan;
+import org.apache.doris.nereids.util.MemoTestUtils;
+import org.apache.doris.planner.PlanFragment;
+import org.apache.doris.planner.PlanNode;
+import org.apache.doris.thrift.TExprNode;
+import org.apache.doris.thrift.TExprNodeType;
+import org.apache.doris.utframe.TestWithFeService;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Verifies that a no-op cast (e.g. `cast(char_col as text)`) is eliminated
during
+ * thrift serialization even when it is a nested child of another expression
+ * (e.g. `cast(a as text) = '1'` should be serialized as `a = '1'`).
+ */
+public class CastExprThriftSimplificationTest extends TestWithFeService {
+
+ @Override
+ protected void runBeforeAll() throws Exception {
+ createDatabase("test");
+ createTable("create table test.t (a char(6), b int) "
+ + "distributed by hash(b) buckets 1 "
+ + "properties('replication_num' = '1');");
+ // Avoid the empty scan being pruned away in the test environment.
+
connectContext.getSessionVariable().setDisableNereidsRules("prune_empty_partition");
+ }
+
+ @Test
+ public void testNoOpCastInConjunctIsEliminatedInThrift() throws Exception {
+ String sql = "select * from test.t where cast(a as text) = '1'";
+ StatementContext statementContext =
MemoTestUtils.createStatementContext(connectContext, sql);
+ PhysicalPlan plan = new NereidsPlanner(statementContext).planWithLock(
+ new NereidsParser().parseSingle(sql),
+ PhysicalProperties.ANY
+ );
+
+ PlanFragment fragment = new PhysicalPlanTranslator(new
PlanTranslatorContext()).translatePlan(plan);
+ PlanNode planNode = fragment.getPlanRoot();
+
+ List<PlanNode> allNodes = new ArrayList<>();
+ planNode.foreachDown(n -> {
+ allNodes.add((PlanNode) n);
+ return true;
+ });
+
+ boolean foundCastExpr = false;
+ for (PlanNode node : allNodes) {
+ for (Expr e : node.getConjuncts()) {
+ for (TExprNode tnode : e.treeToThrift().getNodes()) {
+ if (tnode.getNodeType() == TExprNodeType.CAST_EXPR) {
+ foundCastExpr = true;
+ }
+ }
+ }
+ }
+ Assertions.assertFalse(foundCastExpr,
+ "no-op cast should be eliminated during thrift serialization,
but CAST_EXPR node is present");
+ }
+
+ @Test
+ public void testNonNoOpCastInConjunctIsKeptInThrift() throws Exception {
+ String sql = "select * from test.t where cast(b as text) = '1'";
+ StatementContext statementContext =
MemoTestUtils.createStatementContext(connectContext, sql);
+ PhysicalPlan plan = new NereidsPlanner(statementContext).planWithLock(
+ new NereidsParser().parseSingle(sql),
+ PhysicalProperties.ANY
+ );
+
+ PlanFragment fragment = new PhysicalPlanTranslator(new
PlanTranslatorContext()).translatePlan(plan);
+ PlanNode planNode = fragment.getPlanRoot();
+
+ List<PlanNode> allNodes = new ArrayList<>();
+ planNode.foreachDown(n -> {
+ allNodes.add((PlanNode) n);
+ return true;
+ });
+
+ boolean foundCastExpr = false;
+ for (PlanNode node : allNodes) {
+ for (Expr e : node.getConjuncts()) {
+ for (TExprNode tnode : e.treeToThrift().getNodes()) {
+ if (tnode.getNodeType() == TExprNodeType.CAST_EXPR) {
+ foundCastExpr = true;
+ }
+ }
+ }
+ }
+ Assertions.assertTrue(foundCastExpr,
+ "int -> text cast is a real cast and must be kept during
thrift serialization");
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
