Todd Lipcon has posted comments on this change. Change subject: [timestamp] Add a new TIMESTAMP type ......................................................................
Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/5819/3/src/kudu/client/schema.h File src/kudu/client/schema.h: Line 135: TIMESTAMP = 10 > I think we're good, here's my reasoning: > compilation of old user code against the new client lib would fail since the > setters accept different types. Wouldn't it only fail at runtime? Maybe we should name this new type TIMESTAMP_NANOS anyway, since Impala is likely to eventually add TIMESTAMP(6) with millisecond precision down the line, and we'll want a way to distinguish them? This way we'd avoid another future rename, plus we wouldn't have the back-compat issue. (meanwhile we could now remove the 'TIMESTAMP =' for a couple versions to force people to switch to the new names) http://gerrit.cloudera.org:8080/#/c/5819/3/src/kudu/util/timestamp_value.cc File src/kudu/util/timestamp_value.cc: Line 128: pt += boost::posix_time::nanoseconds(1); > hum apparently tidy bot is not picking up the definition that enables nanos Maybe we should just be #defining it at the top of this file? then tidybot would see it. (we don't expose the ptimes ever in the header, right?) PS3, Line 138: CHECK_OK(ToPTime(&date_, &time_duration_, &pt)); : pt -= boost::posix_time::nanoseconds(1); : if (!pt.is_not_a_date_time()) { : FromPTime(pt, &date_, &time_duration_); : return true; : } : > this is slightly wrong (nanos_ must be set to 86399999999999L) but changed oh, right... i wish days were power-of-ten number of seconds :) -- To view, visit http://gerrit.cloudera.org:8080/5819 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I63271adf6b87f048e30bf2d6d04c758884752afc Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
