Skip to content

[fix](Nereids) ComputePrecisionForRound should only process integral literal #34473

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
morrySnow wants to merge 1 commit into from
Closed

[fix](Nereids) ComputePrecisionForRound should only process integral literal #34473

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions fe/fe-core/src/main/java/org/apache/doris/nereids/NereidsPlanner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,8 +581,7 @@ public Optional<ResultSet> handleQueryInFe(StatementBase parsedStmt) {
return Optional.of(resultSet);
} else if (child instanceof PhysicalEmptyRelation) {
List<Column> columns = Lists.newArrayList();
PhysicalEmptyRelation physicalEmptyRelation = (PhysicalEmptyRelation) physicalPlan.child(0);
for (int i = 0; i < physicalEmptyRelation.getProjects().size(); i++) {
for (int i = 0; i < physicalPlan.getOutput().size(); i++) {
Copy link
Contributor

@zhiqiang-hhhh zhiqiang-hhhh May 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the relation between this for loop with round function?

Copy link
Contributor Author

@morrySnow morrySnow May 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is another bug, and have no relation with round function

NamedExpression output = physicalPlan.getOutput().get(i);
columns.add(new Column(output.getName(), output.getDataType().toCatalogDataType()));
}
Expand Down
3 changes: 2 additions & 1 deletion ...n/java/org/apache/doris/nereids/trees/expressions/functions/ComputePrecisionForRound.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ default FunctionSignature computePrecision(FunctionSignature signature) {
Expression floatLength = getArgument(1);
int scale;

if (floatLength.isLiteral() || (floatLength instanceof Cast && floatLength.child(0).isLiteral()
if ((floatLength.isLiteral() && floatLength.getDataType() instanceof Int32OrLessType)
Copy link
Contributor

@zhiqiang-hhhh zhiqiang-hhhh May 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in what situation would scale does not have integer type?

Copy link
Contributor Author

@morrySnow morrySnow May 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when we do computePrecision, we do not finish compute signature. so type coercion do not apply. so its child could be any type literal

|| (floatLength instanceof Cast && floatLength.child(0).isLiteral()
&& floatLength.child(0).getDataType() instanceof Int32OrLessType)) {
// Scale argument is a literal or cast from other literal
if (floatLength instanceof Cast) {
Expand Down
10 changes: 0 additions & 10 deletions regression-test/suites/correctness_p0/test_cast_decimal.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,6 @@ suite("test_cast_decimal") {
sql """select cast(32123.34212456734 as decimal(3,2));"""
contains "cast(32123.34212456734 as DECIMALV3(3, 2))"
}


sql """
set enable_nereids_planner=false;
"""

explain {
sql """select cast(32123.34212456734 as decimal(3,2));"""
contains "CAST(32123.34212456734 AS DECIMALV3(3, 2))"
}

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");"""
Expand Down