Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14750 )
Change subject: IMPALA-9092: Add support for creating external Kudu table ...................................................................... Patch Set 1: (7 comments) http://gerrit.cloudera.org:8080/#/c/14750/1/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/14750/1/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@75 PS1, Line 75: "TRANSLATED_TO_EXTERNAL" > I don't think it'll be misleading to users since we already show TRANSLATED Thanks for making this a non-blocking suggestion. I would like to defer this to later in the interest of moving forward on this patch since I think if I change it now, I will have to change a some of tests again. I would be happy to submit a separate patch if we see some user interest in this. http://gerrit.cloudera.org:8080/#/c/14750/10/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/14750/10/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@103 PS10, Line 103: "val > nit: looks like indentions are still inconsistent in some lines like this Done http://gerrit.cloudera.org:8080/#/c/14750/10/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@148 PS10, Line 148: // Primary key column that doesn't exist > nit: and this Done http://gerrit.cloudera.org:8080/#/c/14750/10/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@177 PS10, Line 177: "compatible with partitioning column 'a' (type: INT)."); > nit: and this Done http://gerrit.cloudera.org:8080/#/c/14750/10/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@209 PS10, Line 209: AnalysisError("create table tab (a int, b int, primary key (a, b)) " + > nit: and this Done http://gerrit.cloudera.org:8080/#/c/14750/10/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@278 PS10, Line 278: "S > nit: leading comma Done http://gerrit.cloudera.org:8080/#/c/14750/10/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@455 PS10, Line 455: AnalysisError("create table tdefault (id int primary key, " + > line too long (91 > 90) Done -- To view, visit http://gerrit.cloudera.org:8080/14750 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I76f81d41db0cf2269ee1b365857164a43677e14d Gerrit-Change-Number: 14750 Gerrit-PatchSet: 1 Gerrit-Owner: Vihang Karajgaonkar <[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: Joe McDonnell <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]> Gerrit-Comment-Date: Fri, 13 Dec 2019 18:34:05 +0000 Gerrit-HasComments: Yes
