Jim Apple 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 the > callers are scanners, so if the data in the file is corrupt, then it is > possible to get out of range values here. So if we would add a DCHECK, then > some files could crash a debug build. If we add the validation to this function, do you want it added in a non-DCHECK way? Do you just want to return an invalid TimestampValue, rather than crashing? -- 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 <jbapple-imp...@apache.org> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org> Gerrit-Comment-Date: Mon, 11 Feb 2019 16:59:29 +0000 Gerrit-HasComments: Yes
