Alex Behm has posted comments on this change.

Change subject: IMPALA-3437: DECIMAL_V2: avoid implicit decimal->double 
conversion
......................................................................


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/7916/5/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java:

Line 1331:     checkDecimalReturnType("select 1 + 1.1",
> 1 -> interpreted as tinyint -> decimal(3, 0)
Ahh right, thanks for the refresher.


Line 1386:         Type.DOUBLE, ScalarType.createDecimalType(2, 1));
> So I guess the current behaviour seems OK to me but I haven't thought deepl
Agree.


http://gerrit.cloudera.org:8080/#/c/7916/6/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java:

Line 1476:     checkDecimalReturnType("select 
12345678901234567890123456789012345678",
Feels more appropriate to put these into TestNumericLiteralMinMaxValues() at 
the top of this file. There are already some existing tests that we should 
dedup with your new tests. Perhaps some of the existing tests there should also 
use checkDecimalReturnType()


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie419a75784eec2294947103e6e1465dfadfc29da
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Greg Rahn <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to