Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/13318 )
Change subject: IMPALA-8504: Support CREATE TABLE statement with Kudu/HMS integration ...................................................................... Patch Set 7: (5 comments) Looks good, just a couple more nitpicks I think before this can go in http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/main/java/org/apache/impala/catalog/KuduTable.java File fe/src/main/java/org/apache/impala/catalog/KuduTable.java: http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@171 PS7, Line 171: Error getting the configuration on whether I find this awkwardly worded, maybe 'Error determining if' http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@173 PS7, Line 173: Kudu error unnecessary http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java File fe/src/test/java/org/apache/impala/analysis/AuditingTest.java: http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/test/java/org/apache/impala/analysis/AuditingTest.java@56 PS7, Line 56: new TAccessEvent("functional.alltypes", TCatalogObjectType.TABLE, "SELECT"))); Probably better to leave off unrelated formatting changes like this. It makes it more difficult to deal with stuff like backports http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java File fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java: http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/test/java/org/apache/impala/customcluster/CustomClusterRunner.java@49 PS7, Line 49: public static int RestartMiniclusterComponent(String component, This should be removed http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/test/java/org/apache/impala/customservice/CustomServiceRunner.java File fe/src/test/java/org/apache/impala/customservice/CustomServiceRunner.java: http://gerrit.cloudera.org:8080/#/c/13318/7/fe/src/test/java/org/apache/impala/customservice/CustomServiceRunner.java@32 PS7, Line 32: mini cluster minicluster component -- To view, visit http://gerrit.cloudera.org:8080/13318 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I465673d749221bd5f3772814b1c22c2673a53f5c Gerrit-Change-Number: 13318 Gerrit-PatchSet: 7 Gerrit-Owner: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Thomas Marshall <tmarsh...@cloudera.com> Gerrit-Comment-Date: Wed, 29 May 2019 19:37:24 +0000 Gerrit-HasComments: Yes
