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

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


Patch Set 6:

(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_) + ".");
> As of now we don't have test to print the SQL for create or alter view stat
ok, pls add several cases. this was caught by code inspection, so will rot 
as-is. until we decide to disable or remove it, lets capture some basics in 
analysis.ToSqlTest.java


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: *
explicitly list the column using the name in the 'alter' stmt so that there 
isn't a false positive (who knows, perhaps the prev. col name is stuck 
somewhere)



--
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: 6
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 22:01:55 +0000
Gerrit-HasComments: Yes

Reply via email to