impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lars Volker (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-5668: Fix cast(X as timestamp) for negative subsecond Decimals
Date Mon, 09 Oct 2017 19:58:38 GMT
Lars Volker has posted comments on this change. ( )

Change subject: IMPALA-5668: Fix cast(X as timestamp) for negative subsecond Decimals

Patch Set 2:

Commit Message:
PS2, Line 9: The timestamp conversion from negative fractional Decimal types
           : (interpreted as unix timestamp) was wrong - the whole part
           : was rounded toward zero, and fractional part was being added
           : instead of being subtracted.
The commit message should include a brief description of how change fixes it.
PS2, Line 14: Example for the wrong behaivour:
Spelling. Please consider setting up an automatic spell checker in your text editor.
File be/src/exprs/
PS2, Line 617:       if(dv.is_negative()) nanoseconds *= -1;
I think it might be easier to read if you extract the sign in in line 610 and then just multiply
nanoseconds with the sign during its initialization or in the call to FromUnixTimeNanos().
PS2, Line 625: int64_t
why is this 64bit?
PS2, Line 640: int128_t
why is this 128bit?
File be/src/exprs/
PS2, Line 2363:   TestTimestampValue("cast(cast(-123.45 as decimal(9,2)) as timestamp)",
If you include a negative example in the commit message, it should also be included in the

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8216f4c0f100c1bd68891cd6048236bfe4c205f0
Gerrit-Change-Number: 8051
Gerrit-PatchSet: 2
Gerrit-Owner: Csaba Ringhofer <>
Gerrit-Reviewer: Lars Volker <>
Gerrit-Comment-Date: Mon, 09 Oct 2017 19:58:38 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message