-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[Fix](Nereids) fix floor/round/ceil/truncate functions type compute precision problem #43422
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.
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
Conversation
|
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
|
run buildall |
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
| testFoldConst("with cte as (select floor(300.343, 2) order by 1 limit 1) select * from cte") | ||
| testFoldConst("with cte as (select round(300.343, 2) order by 1 limit 1) select * from cte") | ||
| testFoldConst("with cte as (select ceil(300.343, 2) order by 1 limit 1) select * from cte") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested the query before this PR:
with cte as (select truncate(300.343, 2) order by 1 limit 1) select * from cte;,
and the exception is:
ERROR 1105 (HY000): errCode = 2, detailMessage = (9.134.238.135)[INTERNAL_ERROR]output type not match expr type , col name , expected type Decimal(6, 2) , real type Decimal(5, 2).
So whether the truncate() should be tested together?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it make sense, added
c2c876d to
e9abaf6
Compare
|
run buildall |
1 similar comment
|
run buildall |
|
run p0 |
|
run cloud_p0 |
|
run buildall |
| public static Expression truncate(DecimalV3Literal first, IntegerLiteral second) { | ||
| if (first.getValue().compareTo(BigDecimal.ZERO) == 0) { | ||
| return first; | ||
| } else if (first.getValue().compareTo(BigDecimal.ZERO) == -1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } else if (first.getValue().compareTo(BigDecimal.ZERO) == -1) { | |
| } else if (first.getValue().compareTo(BigDecimal.ZERO) < 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the function compareTo only return 0 -1 and 1, so I think -1 is acceptable for this situation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://docs.oracle.com/javase/8/docs/api/java/math/BigDecimal.html
maybe you can check bigDecimal Type implementation for compareTo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed to aligning interface definition
|
run buildall |
3715356 to
309671a
Compare
|
run buildall |
…recision when fold constant
309671a to
84dc072
Compare
|
run buildall |
|
run buildall |
|
PR approved by at least one committer and no changes requested. |
…recision problem (#43422) - Problem function like ```select floor(300.343, 2)``` precision should be 5 and scale should be 2, but now is (6, 2) after compute precision, but after folding const on fe, it changed to (5, 2) but upper level of plan still expect the output of child to be (6, 2). So it would rise an exception when executing. - How it was fixed fix folding constant precision of floor/round/ceil/truncate functions from (5, 2) to (6, 2) in upper case - Notion when second value is negative and it absolute value >= precision - value, it can not be expressed in fe which result is zero with decimal type (3, 0). like 000. So just let it go back and no using folding constant by fe. - Related PR: #40744 - Release note Fix floor/round/ceil functions precision problem in folding constant
…recision problem (#43422) - Problem function like ```select floor(300.343, 2)``` precision should be 5 and scale should be 2, but now is (6, 2) after compute precision, but after folding const on fe, it changed to (5, 2) but upper level of plan still expect the output of child to be (6, 2). So it would rise an exception when executing. - How it was fixed fix folding constant precision of floor/round/ceil/truncate functions from (5, 2) to (6, 2) in upper case - Notion when second value is negative and it absolute value >= precision - value, it can not be expressed in fe which result is zero with decimal type (3, 0). like 000. So just let it go back and no using folding constant by fe. - Related PR: #40744 - Release note Fix floor/round/ceil functions precision problem in folding constant
…recision problem (apache#43422) - Problem function like ```select floor(300.343, 2)``` precision should be 5 and scale should be 2, but now is (6, 2) after compute precision, but after folding const on fe, it changed to (5, 2) but upper level of plan still expect the output of child to be (6, 2). So it would rise an exception when executing. - How it was fixed fix folding constant precision of floor/round/ceil/truncate functions from (5, 2) to (6, 2) in upper case - Notion when second value is negative and it absolute value >= precision - value, it can not be expressed in fe which result is zero with decimal type (3, 0). like 000. So just let it go back and no using folding constant by fe. - Related PR: apache#40744 - Release note Fix floor/round/ceil functions precision problem in folding constant
…recision problem (apache#43422) - Problem function like ```select floor(300.343, 2)``` precision should be 5 and scale should be 2, but now is (6, 2) after compute precision, but after folding const on fe, it changed to (5, 2) but upper level of plan still expect the output of child to be (6, 2). So it would rise an exception when executing. - How it was fixed fix folding constant precision of floor/round/ceil/truncate functions from (5, 2) to (6, 2) in upper case - Notion when second value is negative and it absolute value >= precision - value, it can not be expressed in fe which result is zero with decimal type (3, 0). like 000. So just let it go back and no using folding constant by fe. - Related PR: apache#40744 - Release note Fix floor/round/ceil functions precision problem in folding constant
…recision problem (apache#43422) - Problem function like ```select floor(300.343, 2)``` precision should be 5 and scale should be 2, but now is (6, 2) after compute precision, but after folding const on fe, it changed to (5, 2) but upper level of plan still expect the output of child to be (6, 2). So it would rise an exception when executing. - How it was fixed fix folding constant precision of floor/round/ceil/truncate functions from (5, 2) to (6, 2) in upper case - Notion when second value is negative and it absolute value >= precision - value, it can not be expressed in fe which result is zero with decimal type (3, 0). like 000. So just let it go back and no using folding constant by fe. - Related PR: apache#40744 - Release note Fix floor/round/ceil functions precision problem in folding constant
…ecision problem (apache#43782) Cherry-picked from apache#43422
What problem does this PR solve?
function like
select floor(300.343, 2)precision should be 5 and scale should be 2, but now is (6, 2) after compute precision, but after folding const on fe, it changed to (5, 2) but upper level of plan still expect the output of child to be (6, 2). So it would rise an exception when executing.fix folding constant precision of floor/round/ceil/truncate functions from (5, 2) to (6, 2) in upper case
when second value is negative and it absolute value >= precision - value, it can not be expressed in fe which result is zero with decimal type (3, 0). like 000. So just let it go back and no using folding constant by fe.
Issue Number: close #43421
Related PR: #40744
Problem Summary:
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Release note
Fix floor/round/ceil functions precision problem
Check List (For Reviewer who merge this PR)