Greg Solovyev has posted comments on this change. ( http://gerrit.cloudera.org:8080/13483 )
Change subject: WIP: [KUDU-442] Add Kudu support for Hive ...................................................................... Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/13483/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java File java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java: http://gerrit.cloudera.org:8080/#/c/13483/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java@249 PS1, Line 249: // Preconditions.checkArgument(row.getSchema() == table.getSchema(), Why is this being commented out and what are the effects of this on Kudu java client? http://gerrit.cloudera.org:8080/#/c/13483/1/java/kudu-hive/src/main/java/org/apache/kudu/hive/KuduHiveUtils.java File java/kudu-hive/src/main/java/org/apache/kudu/hive/KuduHiveUtils.java: http://gerrit.cloudera.org:8080/#/c/13483/1/java/kudu-hive/src/main/java/org/apache/kudu/hive/KuduHiveUtils.java@88 PS1, Line 88: LOG.debug("Not importing credentials for service " + service + perhaps this would be slightly more elegant and more informative: LOG.debug("Not importing credentials for service {} (expecting service {})", tok.getService(), service) http://gerrit.cloudera.org:8080/#/c/13483/1/java/kudu-hive/src/main/java/org/apache/kudu/hive/KuduHiveUtils.java@92 PS1, Line 92: LOG.debug("Importing credentials for service " + service); Use format instead of string concatenation? http://gerrit.cloudera.org:8080/#/c/13483/1/java/kudu-hive/src/main/java/org/apache/kudu/hive/KuduInputFormat.java File java/kudu-hive/src/main/java/org/apache/kudu/hive/KuduInputFormat.java: http://gerrit.cloudera.org:8080/#/c/13483/1/java/kudu-hive/src/main/java/org/apache/kudu/hive/KuduInputFormat.java@94 PS1, Line 94: List<String> locations = new ArrayList<>(token.getTablet().getReplicas().size()); nit: I am not sure that using ArrayList<String> instead of simple String[] is reasonable here. Looks like the only advantage is using List.add method without having to keep a counter, while 'locations' has to be converted to String[] anyway when passed to KuduInputSplit -- To view, visit http://gerrit.cloudera.org:8080/13483 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifdcc72c4099dc40a7a2efa59c0c62f63902733d4 Gerrit-Change-Number: 13483 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Greg Solovyev <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 31 May 2019 20:04:58 +0000 Gerrit-HasComments: Yes
