Hao Hao 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: (4 comments) http://gerrit.cloudera.org:8080/#/c/13318/4/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/4/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@192 PS4, Line 192: // TODO: Change this to reflect the number of pk columns and modify all the > I see. I'm fine with keeping this in if it's a hassle for you to rebase, bu LGTM, will pull this change out to the follow up patch where it is used. http://gerrit.cloudera.org:8080/#/c/13318/4/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@200 PS4, Line 200: tch (KuduException e) { > SGTM. Will that be in the follow-up patch where this function is used or a It will be in the follow-up where it is used. http://gerrit.cloudera.org:8080/#/c/13318/6/fe/src/test/java/org/apache/impala/customservice/KuduHMSIntegrationTest.java File fe/src/test/java/org/apache/impala/customservice/KuduHMSIntegrationTest.java: http://gerrit.cloudera.org:8080/#/c/13318/6/fe/src/test/java/org/apache/impala/customservice/KuduHMSIntegrationTest.java@38 PS6, Line 38: AnalyzeKuduDDLTest.class, AuditingKuduTest.class, : ParserTest.class, ToSqlTest.class > Do these also get run without the HMS integration enabled? Yes, it does. http://gerrit.cloudera.org:8080/#/c/13318/6/fe/src/test/java/org/apache/impala/customservice/KuduHMSIntegrationTest.java@60 PS6, Line 60: List<String> envp = new ArrayList<>(); > nit: definition could be gated by enableHMSIntegration if this were put at Done -- 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 00:07:44 +0000 Gerrit-HasComments: Yes
