Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10502 )
Change subject: [java] Add Timestamp APIs to kudu-client ...................................................................... Patch Set 2: (10 comments) http://gerrit.cloudera.org:8080/#/c/10502/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java File java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java: http://gerrit.cloudera.org:8080/#/c/10502/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@557 PS1, Line 557: * @throws IllegalStateException if the row was already applied > Document how java.sql.Timestamp instances with nanosecond precision are tru Done http://gerrit.cloudera.org:8080/#/c/10502/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@566 PS1, Line 566: } > ditto Done http://gerrit.cloudera.org:8080/#/c/10502/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@586 PS1, Line 586: * @param columnName name of the column to get data for > Probably worth documenting that all returned values will be in the UTC time I don't think java.sql.Timestamp objects have time zones. A timezone is only "applied" when formatting a Timestamp (or Date) with something like SimpleDateFormat. http://gerrit.cloudera.org:8080/#/c/10502/1/java/kudu-client/src/main/java/org/apache/kudu/client/PartialRow.java@593 PS1, Line 593: } > ditto Done http://gerrit.cloudera.org:8080/#/c/10502/1/java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java File java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java: http://gerrit.cloudera.org:8080/#/c/10502/1/java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java@354 PS1, Line 354: * @return a Timestamp > Update Done http://gerrit.cloudera.org:8080/#/c/10502/1/java/kudu-client/src/main/java/org/apache/kudu/client/RowResult.java@366 PS1, Line 366: * @return a Timestamp > likewise Done http://gerrit.cloudera.org:8080/#/c/10502/1/java/kudu-client/src/main/java/org/apache/kudu/util/TimestampUtil.java File java/kudu-client/src/main/java/org/apache/kudu/util/TimestampUtil.java: http://gerrit.cloudera.org:8080/#/c/10502/1/java/kudu-client/src/main/java/org/apache/kudu/util/TimestampUtil.java@42 PS1, Line 42: * Converts a {@link Timestamp} to microseconds since the Unix epoch (1970-01-01T00:00:00Z). > I think the brackets are scaladoc specific. Javadoc equivalent is @link or Done http://gerrit.cloudera.org:8080/#/c/10502/1/java/kudu-client/src/main/java/org/apache/kudu/util/TimestampUtil.java@60 PS1, Line 60: > likewise Done http://gerrit.cloudera.org:8080/#/c/10502/1/java/kudu-client/src/main/java/org/apache/kudu/util/TimestampUtil.java@83 PS1, Line 83: * @param micros the timestamp, in microseconds > Shouldn't be necessary to have both versions of this helper anymore, since Done http://gerrit.cloudera.org:8080/#/c/10502/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java: http://gerrit.cloudera.org:8080/#/c/10502/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestPartialRow.java@36 PS1, Line 36: public class TestPartialRow { > Add a tests that writes a timestamp with a non-zulu offset and ensure it's I can write this test in TestTimestampUtil by round-tripping the conversion to and from micros. -- To view, visit http://gerrit.cloudera.org:8080/10502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1fdd397691f12f279663838af9fa12ed27508fd1 Gerrit-Change-Number: 10502 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Comment-Date: Thu, 31 May 2018 15:02:00 +0000 Gerrit-HasComments: Yes
