Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12689 )
Change subject: [java] Add private diff scan support ...................................................................... Patch Set 14: (3 comments) http://gerrit.cloudera.org:8080/#/c/12689/14/java/kudu-client/src/main/java/org/apache/kudu/Schema.java File java/kudu-client/src/main/java/org/apache/kudu/Schema.java: http://gerrit.cloudera.org:8080/#/c/12689/14/java/kudu-client/src/main/java/org/apache/kudu/Schema.java@70 PS14, Line 70: private final boolean hasIsDeleted; : private final int isDeletedIndex; If an index value of -1 means "no index", you shouldn't need both of these. http://gerrit.cloudera.org:8080/#/c/12689/14/java/kudu-client/src/main/java/org/apache/kudu/Schema.java@137 PS14, Line 137: if(column.getWireType() == Common.DataType.IS_DELETED) { Nit: if ( http://gerrit.cloudera.org:8080/#/c/12689/14/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java: http://gerrit.cloudera.org:8080/#/c/12689/14/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@156 PS14, Line 156: static final String DEFAULT_IS_DELETED_COL_NAME = "_kudu_is_deleted"; Could we make the default "is_deleted"? Even that seems pretty collision-proof, and I think it'd be more readable if someone snoops the transfer. -- To view, visit http://gerrit.cloudera.org:8080/12689 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I51b01ae76cd22407df9097b56629ac7262ec2964 Gerrit-Change-Number: 12689 Gerrit-PatchSet: 14 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Greg Solovyev <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Comment-Date: Thu, 21 Mar 2019 21:58:00 +0000 Gerrit-HasComments: Yes
