This is an automated email from the ASF dual-hosted git repository.
dataroaring pushed a commit to branch branch-3.0
in repository https://gitbox.apache.org/repos/asf/doris.git
commit e8b3a8316900f6f5da6201323a44f7a8bb357d3b
Author: morrySnow <101034200+morrys...@users.noreply.github.com>
AuthorDate: Fri Sep 13 10:38:22 2024 +0800
[fix](Nereids) avoid bad cast when compute scale for round (#40776)
for pass test case, also fix errors in computeResultInFe
computeResultInFe will generate wrong result set if output does not
match between final result and the node executing computeResultInFe
---
.../org/apache/doris/nereids/NereidsPlanner.java | 2 +-
.../functions/ComputePrecisionForRound.java | 4 +++-
.../nereids/trees/plans/ComputeResultSet.java | 5 +++-
.../plans/physical/PhysicalEmptyRelation.java | 6 ++---
.../plans/physical/PhysicalOneRowRelation.java | 28 +++++++++++++---------
.../trees/plans/physical/PhysicalResultSink.java | 5 ++--
.../trees/plans/physical/PhysicalSqlCache.java | 2 +-
.../suites/correctness_p0/test_cast_decimal.groovy | 1 -
8 files changed, 31 insertions(+), 22 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java
index 9d32af433b5..9d5bc779874 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java
@@ -642,7 +642,7 @@ public class NereidsPlanner extends Planner {
if (physicalPlan instanceof ComputeResultSet) {
Optional<SqlCacheContext> sqlCacheContext =
statementContext.getSqlCacheContext();
Optional<ResultSet> resultSet = ((ComputeResultSet) physicalPlan)
- .computeResultInFe(cascadesContext, sqlCacheContext);
+ .computeResultInFe(cascadesContext, sqlCacheContext,
physicalPlan.getOutput());
if (resultSet.isPresent()) {
return resultSet;
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/ComputePrecisionForRound.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/ComputePrecisionForRound.java
index b47804e23ff..b07b7d384d8 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/ComputePrecisionForRound.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/ComputePrecisionForRound.java
@@ -40,8 +40,10 @@ public interface ComputePrecisionForRound extends
ComputePrecision {
// If scale arg is an integer literal, or it is a cast(Integer as
Integer)
// then we will try to use its value as result scale
// In any other cases, we will make sure result decimal has same
scale with input.
- if ((floatLength.isLiteral() && floatLength.getDataType()
instanceof Int32OrLessType)
+ if ((floatLength.isLiteral() && !floatLength.isNullLiteral()
+ && floatLength.getDataType() instanceof Int32OrLessType)
|| (floatLength instanceof Cast &&
floatLength.child(0).isLiteral()
+ && !floatLength.child(0).isNullLiteral()
&& floatLength.child(0).getDataType() instanceof
Int32OrLessType)) {
if (floatLength instanceof Cast) {
scale = ((IntegerLikeLiteral)
floatLength.child(0)).getIntValue();
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/ComputeResultSet.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/ComputeResultSet.java
index beee784ec9d..f86e143ca7b 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/ComputeResultSet.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/ComputeResultSet.java
@@ -19,8 +19,10 @@ package org.apache.doris.nereids.trees.plans;
import org.apache.doris.nereids.CascadesContext;
import org.apache.doris.nereids.SqlCacheContext;
+import org.apache.doris.nereids.trees.expressions.Slot;
import org.apache.doris.qe.ResultSet;
+import java.util.List;
import java.util.Optional;
/**
@@ -51,5 +53,6 @@ import java.util.Optional;
* </pre>
*/
public interface ComputeResultSet {
- Optional<ResultSet> computeResultInFe(CascadesContext cascadesContext,
Optional<SqlCacheContext> sqlCacheContext);
+ Optional<ResultSet> computeResultInFe(CascadesContext cascadesContext,
Optional<SqlCacheContext> sqlCacheContext,
+ List<Slot> outputSlots);
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalEmptyRelation.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalEmptyRelation.java
index e01c3ead327..30beb0d2f41 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalEmptyRelation.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalEmptyRelation.java
@@ -116,11 +116,9 @@ public class PhysicalEmptyRelation extends
PhysicalRelation implements EmptyRela
@Override
public Optional<ResultSet> computeResultInFe(CascadesContext
cascadesContext,
- Optional<SqlCacheContext> sqlCacheContext) {
+ Optional<SqlCacheContext> sqlCacheContext, List<Slot> outputSlots)
{
List<Column> columns = Lists.newArrayList();
- List<Slot> outputSlots = getOutput();
- for (int i = 0; i < outputSlots.size(); i++) {
- NamedExpression output = outputSlots.get(i);
+ for (NamedExpression output : outputSlots) {
columns.add(new Column(output.getName(),
output.getDataType().toCatalogDataType()));
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalOneRowRelation.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalOneRowRelation.java
index cd068316b8c..e3c1ca7f493 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalOneRowRelation.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalOneRowRelation.java
@@ -28,6 +28,7 @@ import org.apache.doris.nereids.properties.LogicalProperties;
import org.apache.doris.nereids.properties.PhysicalProperties;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.Slot;
import org.apache.doris.nereids.trees.expressions.literal.Literal;
import org.apache.doris.nereids.trees.plans.ComputeResultSet;
import org.apache.doris.nereids.trees.plans.Plan;
@@ -136,19 +137,24 @@ public class PhysicalOneRowRelation extends
PhysicalRelation implements OneRowRe
@Override
public Optional<ResultSet> computeResultInFe(
- CascadesContext cascadesContext, Optional<SqlCacheContext>
sqlCacheContext) {
+ CascadesContext cascadesContext, Optional<SqlCacheContext>
sqlCacheContext, List<Slot> outputSlots) {
List<Column> columns = Lists.newArrayList();
List<String> data = Lists.newArrayList();
- for (int i = 0; i < projects.size(); i++) {
- NamedExpression item = projects.get(i);
- NamedExpression output = getOutput().get(i);
- Expression expr = item.child(0);
- if (expr instanceof Literal) {
- LiteralExpr legacyExpr = ((Literal) expr).toLegacyLiteral();
- columns.add(new Column(output.getName(),
output.getDataType().toCatalogDataType()));
-
data.add(legacyExpr.getStringValueInFe(cascadesContext.getStatementContext().getFormatOptions()));
- } else {
- return Optional.empty();
+ for (Slot outputSlot : outputSlots) {
+ for (int i = 0; i < projects.size(); i++) {
+ NamedExpression item = projects.get(i);
+ NamedExpression output = getOutput().get(i);
+ if (!outputSlot.getExprId().equals(output.getExprId())) {
+ continue;
+ }
+ Expression expr = item.child(0);
+ if (expr instanceof Literal) {
+ LiteralExpr legacyExpr = ((Literal)
expr).toLegacyLiteral();
+ columns.add(new Column(output.getName(),
output.getDataType().toCatalogDataType()));
+
data.add(legacyExpr.getStringValueInFe(cascadesContext.getStatementContext().getFormatOptions()));
+ } else {
+ return Optional.empty();
+ }
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalResultSink.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalResultSink.java
index 8fb6dfb286e..46df134c0cd 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalResultSink.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalResultSink.java
@@ -24,6 +24,7 @@ import org.apache.doris.nereids.properties.LogicalProperties;
import org.apache.doris.nereids.properties.PhysicalProperties;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.NamedExpression;
+import org.apache.doris.nereids.trees.expressions.Slot;
import org.apache.doris.nereids.trees.plans.ComputeResultSet;
import org.apache.doris.nereids.trees.plans.Plan;
import org.apache.doris.nereids.trees.plans.PlanType;
@@ -129,10 +130,10 @@ public class PhysicalResultSink<CHILD_TYPE extends Plan>
extends PhysicalSink<CH
@Override
public Optional<ResultSet> computeResultInFe(
- CascadesContext cascadesContext, Optional<SqlCacheContext>
sqlCacheContext) {
+ CascadesContext cascadesContext, Optional<SqlCacheContext>
sqlCacheContext, List<Slot> outputSlots) {
CHILD_TYPE child = child();
if (child instanceof ComputeResultSet) {
- return ((ComputeResultSet)
child).computeResultInFe(cascadesContext, sqlCacheContext);
+ return ((ComputeResultSet)
child).computeResultInFe(cascadesContext, sqlCacheContext, outputSlots);
} else {
return Optional.empty();
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalSqlCache.java
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalSqlCache.java
index 549b70a296d..f0fde282011 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalSqlCache.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/physical/PhysicalSqlCache.java
@@ -167,7 +167,7 @@ public class PhysicalSqlCache extends PhysicalLeaf
implements SqlCache, TreeStri
@Override
public Optional<ResultSet> computeResultInFe(
- CascadesContext cascadesContext, Optional<SqlCacheContext>
sqlCacheContext) {
+ CascadesContext cascadesContext, Optional<SqlCacheContext>
sqlCacheContext, List<Slot> outputSlots) {
return resultSet;
}
}
diff --git a/regression-test/suites/correctness_p0/test_cast_decimal.groovy
b/regression-test/suites/correctness_p0/test_cast_decimal.groovy
index 17575fa0aa1..88f127606ed 100644
--- a/regression-test/suites/correctness_p0/test_cast_decimal.groovy
+++ b/regression-test/suites/correctness_p0/test_cast_decimal.groovy
@@ -23,7 +23,6 @@ suite("test_cast_decimal") {
sql """drop table if exists test_ttt"""
sql """create table test_ttt(big_key bigint)DISTRIBUTED BY HASH(big_key)
BUCKETS 1 PROPERTIES ("replication_num" = "1");"""
- sql """set enable_nereids_planner=false;"""
sql """set enable_fold_constant_by_be = false; """
sql """SELECT 1
FROM test_ttt e1
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
