Alexey Serbin 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 3: (14 comments) http://gerrit.cloudera.org:8080/#/c/13318/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13318/3//COMMIT_MSG@9 PS3, Line 9: commits commit http://gerrit.cloudera.org:8080/#/c/13318/3//COMMIT_MSG@9 PS3, Line 9: support adds support for http://gerrit.cloudera.org:8080/#/c/13318/3//COMMIT_MSG@11 PS3, Line 11: actaul actual http://gerrit.cloudera.org:8080/#/c/13318/3//COMMIT_MSG@13 PS3, Line 13: managed a managed http://gerrit.cloudera.org:8080/#/c/13318/3//COMMIT_MSG@19 PS3, Line 19: the naming convention 'db_name.table_name' instead of : 'impala::db_name.table_name'. BTW, what happens with the name of the table created in Kudu when the integration with HMS was enabled upon toggling off HMS integration? Will it be re-named by automatically or by one of the tools? http://gerrit.cloudera.org:8080/#/c/13318/3//COMMIT_MSG@23 PS3, Line 23: commits commit http://gerrit.cloudera.org:8080/#/c/13318/3//COMMIT_MSG@23 PS3, Line 23: Kudu related nit: Kudu-related http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java: http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@329 PS3, Line 329: () nit: drop? http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@329 PS3, Line 329: managed nit: a managed Kudu http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java: http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@440 PS3, Line 440: STORED AS KUDU Is it possible to create a Kudu table with STORED AS KUDU syntax and simultaneously specifying the appropriate/non-appropriate 'storage_handle' property? http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@485 PS3, Line 485: an Kudu table nit: a Kudu table (should be fixed in corresponding place) http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java File fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java: http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java@22 PS3, Line 22: TestKuduStatements What about ALTER TABLE? If generating auditing events for ALTER is not supported explicitly, please add a comment about that. http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java@24 PS3, Line 24: // Select What about CTAS statement? Do we expect both CREATE and SELECT audit events to be issued? http://gerrit.cloudera.org:8080/#/c/13318/3/fe/src/test/java/org/apache/impala/analysis/AuditingKuduTest.java@81 PS3, Line 81: // Drop table : accessEvents = AnalyzeAccessEvents("drop table functional_kudu.testtbl"); : Assert.assertEquals(accessEvents, Sets.newHashSet(new TAccessEvent( : "functional_kudu.testtbl", TCatalogObjectType.TABLE, "DROP"))); What about 'DROP TABLE ... IF EXISTS'? -- 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: 3 Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Thomas Marshall <[email protected]> Gerrit-Comment-Date: Tue, 21 May 2019 00:46:50 +0000 Gerrit-HasComments: Yes
