impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4809: Enable support for DECIMAL V2 in decimal
Date Tue, 14 Feb 2017 23:39:48 GMT
Dan Hecht has posted comments on this change.

Change subject: IMPALA-4809: Enable support for DECIMAL_V2 in

Patch Set 1:

File tests/query_test/

PS1, Line 34: casts from floats
is this true? i don't see where it casts from float to decimal. maybe this comment predates
a change to make literals treated as decimal.
also, this test does test cast decimal to decimal, right? can you update this comment to explain
what it covers?
let's add a TODO for testing float/double to decimal.

PS1, Line 48: precisin

Line 79:     assert expected == actual, "Cast: {0}, Expected: {1}, Actual: {2}".format(cast,\
> +1 internets

Line 83:     # TODO: Fix casting from double to decimal for precision > 38.
what does this mean? today it truncates on double to decimal cast.

PS1, Line 161: is_decimal_v2
how about:

is_decimal_v2 and cast_from != "string"

and then delete line 151-152 so we still exercise string in v2? Then when we do cast from
string with rounding we just delete the second clause here?

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Icf2c8c9d360ad92cbdc5ce902ee742ec4408a8a3
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <>
Gerrit-Reviewer: Dan Hecht <>
Gerrit-Reviewer: Michael Ho <>
Gerrit-Reviewer: Zach Amsden <>
Gerrit-HasComments: Yes

View raw message