Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/11919 )
Change subject: IMPALA-5031: signed overflow in TimestampValue ...................................................................... Patch Set 3: > I like the current solution, but I think that it should break > several tests in timestamp-test.cc, for example > https://github.com/apache/impala/blob/ecf12bec42e11262b88dc0993e375fe4d8acaafb/be/src/runtime/timestamp-test.cc#L290 Earlier, you said, "I would prefer to avoid the overflow by changing the interface to handle nanos only in the -999'999'999 .. + 999'999'999 range, and treat other values as invalid timestamps." I gather from this that you would prefer to alter the test to meet this restriction than you would to widen the allowed values in FromUnixTimeNanos? -- 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: 3 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 Mar 2019 22:37:31 +0000 Gerrit-HasComments: No