Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-3719: Simplify CREATE TABLE statements with Kudu tables ......................................................................
Patch Set 9: Code-Review+1 (9 comments) Carry mikeb's +1 for python tests. http://gerrit.cloudera.org:8080/#/c/4414/7/be/src/service/frontend.cc File be/src/service/frontend.cc: Line 62: "value should be a comma separated list of hostnames or IP addresses."); > Add that detail to the description. Done http://gerrit.cloudera.org:8080/#/c/4414/9/fe/src/main/cup/sql-parser.cup File fe/src/main/cup/sql-parser.cup: Line 1047: // Used for creating tables where the schema is inferred externally, e.g., from an Avro > whitespace Done http://gerrit.cloudera.org:8080/#/c/4414/7/fe/src/main/java/org/apache/impala/analysis/DistributeParam.java File fe/src/main/java/org/apache/impala/analysis/DistributeParam.java: Line 105: for (String colName: colNames_) { > I was wondering whether we should allow something like DISTRIBUTED BY(a, a, Actually this case is rejected by Kudu. Added a test in kudu_create.test. http://gerrit.cloudera.org:8080/#/c/4414/6/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java: Line 1350: // CTAS in an external Kudu table > I was thinking it would mean the same thing it does for HDFS tables. After I see. Let's defer this to a follow up patch if you don't mind. Besides there are other CREATE TABLE statements that we need to revisit for Kudu such as CREATE TABLE LIKE. As is right now, this change would require non-substantial changes to the way we analyze and execute this statement. Filed a JIRA and added a TODO. http://gerrit.cloudera.org:8080/#/c/4414/9/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java: Line 1719: // bucket is partitioned into 4 tables based on the split points of 'y'. > tables == tablets? :) yes. Done http://gerrit.cloudera.org:8080/#/c/4414/6/testdata/bin/generate-schema-statements.py File testdata/bin/generate-schema-statements.py: Line 203: if row_format and file_format == 'text': > There are a few examples here: That's fine. I changed it to be on the safe side. http://gerrit.cloudera.org:8080/#/c/4414/9/testdata/workloads/functional-query/queries/QueryTest/kudu_create.test File testdata/workloads/functional-query/queries/QueryTest/kudu_create.test: Line 82: create table `add`(`analytic` int primary key) distribute by hash (`analytic`) > also cover the range() clause in this same test Done http://gerrit.cloudera.org:8080/#/c/4414/7/tests/query_test/test_kudu.py File tests/query_test/test_kudu.py: Line 204: """ > File JIRA and defer. Done KUDU-1711 Line 213: ) > Thanks for the explanation, seems fine. Done -- To view, visit http://gerrit.cloudera.org:8080/4414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7b9d51b2720ab57649abdb7d5c710ea04ff50dc1 Gerrit-PatchSet: 9 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Michael Brown <mi...@cloudera.com> Gerrit-HasComments: Yes
