Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11919 )
Change subject: IMPALA-5031: signed overflow in TimestampValue ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11919/1/be/src/runtime/timestamp-value.inline.h File be/src/runtime/timestamp-value.inline.h: http://gerrit.cloudera.org:8080/#/c/11919/1/be/src/runtime/timestamp-value.inline.h@66 PS1, Line 66: int64_t nanos > > I prefer the comment + adding the validation to this function. Some of th Yes, I would add it in a non-DCHECK way. Callers that expect the function to be successful could add a DCHECK on the validity of TimestampValue. -- To view, visit http://gerrit.cloudera.org:8080/11919 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaad158e6634314a5690a43a0cc04426c1aba8f41 Gerrit-Change-Number: 11919 Gerrit-PatchSet: 1 Gerrit-Owner: Jim Apple <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Comment-Date: Wed, 13 Feb 2019 12:36:50 +0000 Gerrit-HasComments: Yes
