impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Zach Amsden (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Date Wed, 15 Mar 2017 21:57:33 GMT
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 <zamsden@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-Reviewer: Zach Amsden <zamsden@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message