From 3336e6acbb00c7b90a761000a3d3ff5b934e82a2 Mon Sep 17 00:00:00 2001 From: morrySnow <101034200+morrySnow@users.noreply.github.com> Date: Wed, 18 Sep 2024 13:48:01 +0800 Subject: [PATCH] [fix](Nereids) avoid bad cast when compute scale for round (#40776) (#40904) pick from master #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 --- .../apache/doris/nereids/NereidsPlanner.java | 2 +- .../functions/ComputePrecisionForRound.java | 4 ++- .../nereids/trees/plans/ComputeResultSet.java | 5 +++- .../plans/physical/PhysicalEmptyRelation.java | 6 ++-- .../physical/PhysicalOneRowRelation.java | 28 +++++++++++-------- .../plans/physical/PhysicalResultSink.java | 5 ++-- .../plans/physical/PhysicalSqlCache.java | 2 +- .../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 232ec168b3..36ca1fa099 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 @@ -579,7 +579,7 @@ public class NereidsPlanner extends Planner { if (physicalPlan instanceof ComputeResultSet) { Optional sqlCacheContext = statementContext.getSqlCacheContext(); Optional 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 b47804e23f..b07b7d384d 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 beee784ec9..f86e143ca7 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; * */ public interface ComputeResultSet { - Optional computeResultInFe(CascadesContext cascadesContext, Optional sqlCacheContext); + Optional computeResultInFe(CascadesContext cascadesContext, Optional sqlCacheContext, + List 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 e01c3ead32..30beb0d2f4 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 computeResultInFe(CascadesContext cascadesContext, - Optional sqlCacheContext) { + Optional sqlCacheContext, List outputSlots) { List columns = Lists.newArrayList(); - List 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 cd068316b8..e3c1ca7f49 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 computeResultInFe( - CascadesContext cascadesContext, Optional sqlCacheContext) { + CascadesContext cascadesContext, Optional sqlCacheContext, List outputSlots) { List columns = Lists.newArrayList(); List 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 8fb6dfb286..46df134c0c 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 extends PhysicalSink computeResultInFe( - CascadesContext cascadesContext, Optional sqlCacheContext) { + CascadesContext cascadesContext, Optional sqlCacheContext, List 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 22ff4fdcaa..22f0601705 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 computeResultInFe( - CascadesContext cascadesContext, Optional sqlCacheContext) { + CascadesContext cascadesContext, Optional sqlCacheContext, List 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 17575fa0aa..88f127606e 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