Pooja Nilangekar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10720 )

Change subject: IMPALA-6305: Allow column definitions in ALTER VIEW
......................................................................


Patch Set 5:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/10720/3/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java:

http://gerrit.cloudera.org:8080/#/c/10720/3/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java@76
PS3, Line 76:     if (columnDefs_ != null) sb.append("(" + Joiner.on(", 
").join(columnDefs_) + ".");
> I don't know if we have an existing test to print the SQL. If we do, we sho
As of now we don't have test to print the SQL for create or alter view 
statements.


http://gerrit.cloudera.org:8080/#/c/10720/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java:

http://gerrit.cloudera.org:8080/#/c/10720/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@1102
PS3, Line 1102: +
> nit: + before a new line
Done


http://gerrit.cloudera.org:8080/#/c/10720/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@1106
PS3, Line 1106: +
> nit: + before a new line
Done


http://gerrit.cloudera.org:8080/#/c/10720/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@1110
PS3, Line 1110: +
> nit: + before a new line
Done


http://gerrit.cloudera.org:8080/#/c/10720/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@1159
PS3, Line 1159: "sel
> nit: fix indentation
Done


http://gerrit.cloudera.org:8080/#/c/10720/3/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@1160
PS3, Line 1160: "inn
> nit: fix indentation
Done


http://gerrit.cloudera.org:8080/#/c/10720/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java:

http://gerrit.cloudera.org:8080/#/c/10720/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1660
PS3, Line 1660:     AuthzOk("ALTER VIEW functional.alltypes_view (a, b, c) as " 
+
> nit: is this a tab character? If yes, please use spaces?
Done


http://gerrit.cloudera.org:8080/#/c/10720/3/testdata/workloads/functional-query/queries/QueryTest/views-ddl.test
File testdata/workloads/functional-query/queries/QueryTest/views-ddl.test:

http://gerrit.cloudera.org:8080/#/c/10720/3/testdata/workloads/functional-query/queries/QueryTest/views-ddl.test@234
PS3, Line 234: comment 'abc'
> Unfortunately "describe" doesn't show comments. "describe formatted/extende
Actually, the describe statement does show the comments.  I am currently 
checking it in line 246 - 248. I verified that other tests in views-ddl.test 
also use describe to show the column name, type and comment. Should I 
explicitly change it for this test and the rest of the tests?



-- 
To view, visit http://gerrit.cloudera.org:8080/10720
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6073444a814a24d97e80df15fcd39be2812f63fc
Gerrit-Change-Number: 10720
Gerrit-PatchSet: 5
Gerrit-Owner: Pooja Nilangekar <[email protected]>
Gerrit-Reviewer: Fredy Wijaya <[email protected]>
Gerrit-Reviewer: Pooja Nilangekar <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Vuk Ercegovac <[email protected]>
Gerrit-Comment-Date: Mon, 25 Jun 2018 18:39:42 +0000
Gerrit-HasComments: Yes

Reply via email to