You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "Mryange (via GitHub)" <gi...@apache.org> on 2023/05/22 07:56:05 UTC
[GitHub] [doris] Mryange opened a new pull request, #19926: [fix](DECIMALV3) Fix the error in DECIMALV3 when explicitly casting.
Mryange opened a new pull request, #19926:
URL: https://github.com/apache/doris/pull/19926
# Proposed changes
before
```
mysql [test]>select cast(1 as DECIMALV3(16, 2)) / cast(3 as DECIMALV3(16, 2));
+-----------------------------------------------------------+
| CAST(1 AS DECIMALV3(16, 2)) / CAST(3 AS DECIMALV3(16, 2)) |
+-----------------------------------------------------------+
| 0.00 |
+-----------------------------------------------------------+
mysql [test]>select * from divtest;
+------+------+
| id | val |
+------+------+
| 3 | 5.00 |
| 2 | 4.00 |
| 1 | 3.00 |
+------+------+
mysql [test]>select cast(1 as decimalv3(16,2)) / val from divtest;
+-------------------------------------+
| CAST(1 AS DECIMALV3(16, 2)) / `val` |
+-------------------------------------+
| 0 |
| 0 |
| 0 |
+-------------------------------------+
```
after
```
mysql [test]>select cast(1 as DECIMALV3(16, 2)) / cast(3 as DECIMALV3(16, 2));
+-----------------------------------------------------------+
| CAST(1 AS DECIMALV3(16, 2)) / CAST(3 AS DECIMALV3(16, 2)) |
+-----------------------------------------------------------+
| 0.33 |
+-----------------------------------------------------------+
mysql [test]>select cast(1 as decimalv3(16,2)) / val from divtest;
+-------------------------------------+
| CAST(1 AS DECIMALV3(16, 2)) / `val` |
+-------------------------------------+
| 0.250000 |
| 0.200000 |
| 0.333333 |
+-------------------------------------+
```
This is because in the previous code, the constant 1.000 would be transformed into 1.
Therefore, an additional check is added here. If the constant is converted through casting, "ReduceType" should not be applied.
## Problem summary
Describe your changes.
## Checklist(Required)
* [ ] Does it affect the original behavior
* [ ] Has unit tests been added
* [ ] Has document been added or modified
* [ ] Does it need to update dependencies
* [ ] Is this PR support rollback (If NO, please explain WHY)
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morrySnow commented on pull request #19926: [fix](DECIMALV3) Fix the error in DECIMALV3 when explicitly casting.
Posted by "morrySnow (via GitHub)" <gi...@apache.org>.
morrySnow commented on PR #19926:
URL: https://github.com/apache/doris/pull/19926#issuecomment-1558434078
btw, the divide result should be decimal(18, 2), but current result's scale is 6
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #19926: [fix](DECIMALV3) Fix the error in DECIMALV3 when explicitly casting.
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #19926:
URL: https://github.com/apache/doris/pull/19926#issuecomment-1558425111
PR approved by anyone and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morrySnow commented on a diff in pull request #19926: [fix](DECIMALV3) Fix the error in DECIMALV3 when explicitly casting.
Posted by "morrySnow (via GitHub)" <gi...@apache.org>.
morrySnow commented on code in PR #19926:
URL: https://github.com/apache/doris/pull/19926#discussion_r1201443761
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/Expr.java:
##########
@@ -284,6 +284,11 @@ public boolean apply(Expr arg) {
// Needed for properly capturing expr precedences in the SQL string.
protected boolean printSqlInParens = false;
+ // The code is used to distinguish whether an expression is explicitly cast or
+ // not. For example, different treatments should be applied to a computed value
+ // of 1.000 and a value of 1.000 obtained through casting.
+ protected boolean isCasted = false;
Review Comment:
should set back to default value in `reset()` function
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #19926: [fix](DECIMALV3) Fix the error in DECIMALV3 when explicitly casting.
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #19926:
URL: https://github.com/apache/doris/pull/19926#issuecomment-1558425076
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Mryange commented on pull request #19926: [fix](DECIMALV3) Fix the error in DECIMALV3 when explicitly casting.
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #19926:
URL: https://github.com/apache/doris/pull/19926#issuecomment-1556720149
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] starocean999 commented on a diff in pull request #19926: [fix](DECIMALV3) Fix the error in DECIMALV3 when explicitly casting.
Posted by "starocean999 (via GitHub)" <gi...@apache.org>.
starocean999 commented on code in PR #19926:
URL: https://github.com/apache/doris/pull/19926#discussion_r1201578847
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/ArithmeticExpr.java:
##########
@@ -536,7 +536,7 @@ private void analyzeDecimalV3Op(Type t1, Type t2) throws AnalysisException {
public void analyzeImpl(Analyzer analyzer) throws AnalysisException {
if (VectorizedUtil.isVectorized()) {
for (Expr child : children) {
- if (child instanceof DecimalLiteral && child.getType().isDecimalV3()) {
+ if (child instanceof DecimalLiteral && child.getType().isDecimalV3() && !child.getIsCasted()) {
((DecimalLiteral) child).tryToReduceType();
Review Comment:
try sql `select 1.0000000 / cast(3 as DECIMALV3(1, 0));` seems this pr can't handle such case, IMO,`tryToReduceType()` method is ambiguous, it's not clear why need this function, is it possible to remove it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Mryange commented on a diff in pull request #19926: [fix](DECIMALV3) Fix the error in DECIMALV3 when explicitly casting.
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on code in PR #19926:
URL: https://github.com/apache/doris/pull/19926#discussion_r1201600387
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/ArithmeticExpr.java:
##########
@@ -536,7 +536,7 @@ private void analyzeDecimalV3Op(Type t1, Type t2) throws AnalysisException {
public void analyzeImpl(Analyzer analyzer) throws AnalysisException {
if (VectorizedUtil.isVectorized()) {
for (Expr child : children) {
- if (child instanceof DecimalLiteral && child.getType().isDecimalV3()) {
+ if (child instanceof DecimalLiteral && child.getType().isDecimalV3() && !child.getIsCasted()) {
((DecimalLiteral) child).tryToReduceType();
Review Comment:
I asked the author about this code, and they said it was meant to reduce precision during calculations. now it seems better to remove it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] yiguolei merged pull request #19926: [fix](DECIMALV3) Fix the error in DECIMALV3 when explicitly casting.
Posted by "yiguolei (via GitHub)" <gi...@apache.org>.
yiguolei merged PR #19926:
URL: https://github.com/apache/doris/pull/19926
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #19926: [fix](DECIMALV3) Fix the error in DECIMALV3 when explicitly casting.
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #19926:
URL: https://github.com/apache/doris/pull/19926#issuecomment-1566400058
PR approved by at least one committer and no changes requested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Mryange commented on pull request #19926: [fix](DECIMALV3) Fix the error in DECIMALV3 when explicitly casting.
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #19926:
URL: https://github.com/apache/doris/pull/19926#issuecomment-1559010119
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Mryange commented on pull request #19926: [fix](DECIMALV3) Fix the error in DECIMALV3 when explicitly casting.
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #19926:
URL: https://github.com/apache/doris/pull/19926#issuecomment-1560246084
run p1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Mryange commented on pull request #19926: [fix](DECIMALV3) Fix the error in DECIMALV3 when explicitly casting.
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #19926:
URL: https://github.com/apache/doris/pull/19926#issuecomment-1559367239
run p0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morrySnow commented on a diff in pull request #19926: [fix](DECIMALV3) Fix the error in DECIMALV3 when explicitly casting.
Posted by "morrySnow (via GitHub)" <gi...@apache.org>.
morrySnow commented on code in PR #19926:
URL: https://github.com/apache/doris/pull/19926#discussion_r1201447962
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/Expr.java:
##########
@@ -284,6 +284,11 @@ public boolean apply(Expr arg) {
// Needed for properly capturing expr precedences in the SQL string.
protected boolean printSqlInParens = false;
+ // The code is used to distinguish whether an expression is explicitly cast or
+ // not. For example, different treatments should be applied to a computed value
+ // of 1.000 and a value of 1.000 obtained through casting.
+ protected boolean isCasted = false;
Review Comment:
should set back to default value in `reset()` function
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Mryange commented on pull request #19926: [fix](DECIMALV3) Fix the error in DECIMALV3 when explicitly casting.
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #19926:
URL: https://github.com/apache/doris/pull/19926#issuecomment-1559261186
run p0
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] Mryange commented on pull request #19926: [fix](DECIMALV3) Fix the error in DECIMALV3 when explicitly casting.
Posted by "Mryange (via GitHub)" <gi...@apache.org>.
Mryange commented on PR #19926:
URL: https://github.com/apache/doris/pull/19926#issuecomment-1558799616
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org