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 7:

(2 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_) + ")");
> ok, pls add several cases. this was caught by code inspection, so will rot
As discussed, the current implementation of the toSql function in 
CreateViewStmt and AlterViewStmt is invalid. I have created a jira for this 
(IMPALA-7216). I will fix it and tests in a separate change.


http://gerrit.cloudera.org:8080/#/c/10720/6/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/6/testdata/workloads/functional-query/queries/QueryTest/views-ddl.test@279
PS6, Line 279: f
> explicitly list the column using the name in the 'alter' stmt so that there
Done.



--
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: 7
Gerrit-Owner: Pooja Nilangekar <pooja.nilange...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <pooja.nilange...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Jun 2018 23:42:49 +0000
Gerrit-HasComments: Yes

Reply via email to