Steve Carlin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22086 )

Change subject: IMPALA-13523: Decimal Precision needs to be in return type
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/22086/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/22086/2//COMMIT_MSG@7
PS2, Line 7: IMPALA-13523: Decimal Precision needs to be in return type
> Please update both the Jira and this commit message about what type of test
Done


http://gerrit.cloudera.org:8080/#/c/22086/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/CommonOperatorFunctions.java
File 
java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/CommonOperatorFunctions.java:

http://gerrit.cloudera.org:8080/#/c/22086/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/CommonOperatorFunctions.java@136
PS2, Line 136:     for (RelDataType r : operandTypes) {
             :       if (r.getSqlTypeName() == SqlTypeName.DECIMAL) {
             :         return r;
             :       }
             :     }
> Let me check my understanding: Using the first decimal operand is fine beca
Errr...well....actually....

This is meant for more of the generic functions that exist, like round, or 
appx_median.  There are many functions that just take one decimal parameter and 
do something with it.

There are, of course, some functions that take multiple parameters like "+" and 
all the binary arithmetic operations.  And there's also "case".  But these are 
special-cased, and handled differently.

I suppose it might make more sense to grab a "compatible" type across all 
decimals.  Or at least assert here that there is only one type of decimal used 
in this case.

Thoughts on this?



--
To view, visit http://gerrit.cloudera.org:8080/22086
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie10521b587a74930a01c08b711364f897bb2dc33
Gerrit-Change-Number: 22086
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Steve Carlin <[email protected]>
Gerrit-Comment-Date: Wed, 22 Jan 2025 23:52:53 +0000
Gerrit-HasComments: Yes

Reply via email to