Zach Amsden has posted comments on this change. Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT ......................................................................
Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/6261/6/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java: PS6, Line 201: Ingested > how about analyzeExistingKuduTableParams ? Done http://gerrit.cloudera.org:8080/#/c/6261/6/fe/src/test/java/org/apache/impala/analysis/ParserTest.java File fe/src/test/java/org/apache/impala/analysis/ParserTest.java: Line 2793: > nit extra line Done http://gerrit.cloudera.org:8080/#/c/6261/6/tests/query_test/test_kudu.py File tests/query_test/test_kudu.py: PS6, Line 360: kudu_tbl_name = KuduTestSuite.to_kudu_table_name(unique_database, "foo2") : assert kudu_client.table_exists(kudu_tbl_name) : cursor.execute("SELECT * FROM %s.foo2" % (unique_database)) : assert len(cursor.fetchall()) == 1 : cursor.execute("DROP TABLE %s.foo2" % (unique_database)) : assert kudu_client.table_exists(kudu_tbl_name) > I think you wanna wrap this in a try/finally and make sure to drop the kudu Unless I'm mistaken, the entire unique_database gets dropped at the end of the test. The only reason I'm dropping the tables at all here is to test the external vs. managed table behavior. See for example test_kudu_col_removed, test_kudu_rename_table which don't bother to clean up. -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden <zams...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Zach Amsden <zams...@cloudera.com> Gerrit-HasComments: Yes
