Bharath Vissapragada has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10908 )

Change subject: IMPALA-7209: Disallow self referencing in ALTER VIEW statements
......................................................................


Patch Set 7:

(7 comments)

Looks pretty close.

http://gerrit.cloudera.org:8080/#/c/10908/7/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/10908/7/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java@21
PS7, Line 21: import java.util.LinkedList;
Remove


http://gerrit.cloudera.org:8080/#/c/10908/7/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java@24
PS7, Line 24: import org.apache.impala.analysis.UnionStmt.UnionOperand;
Remove


http://gerrit.cloudera.org:8080/#/c/10908/7/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java
File fe/src/main/java/org/apache/impala/analysis/QueryStmt.java:

http://gerrit.cloudera.org:8080/#/c/10908/7/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@138
PS7, Line 138: FeView
Just curious, why can't we collect Set<InlineViewRef> directly?


http://gerrit.cloudera.org:8080/#/c/10908/7/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java:

http://gerrit.cloudera.org:8080/#/c/10908/7/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@147
PS7, Line 147:   // Returns the QueryStmt present in the whereClause_ of the 
SelectStmt.
...Null otherwise...


http://gerrit.cloudera.org:8080/#/c/10908/7/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@148
PS7, Line 148: getWhereQueryStmt
I think getWhereSubQueryStmt() makes more sense. Also make it private?


http://gerrit.cloudera.org:8080/#/c/10908/7/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@1076
PS7, Line 1076:   public void collectInlineViewRefs(Set<FeView> inlineViewRefs) 
{
nit: Mention that currently only from/where subqueries are supported. Other 
clauses like having.... etc are not allowed.


http://gerrit.cloudera.org:8080/#/c/10908/7/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@1077
PS7, Line 1077:     List<TableRef> fromTblRefs = getTableRefs();
Preconditions.checkNotNull(inlineViewRefs);



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7
Gerrit-Change-Number: 10908
Gerrit-PatchSet: 7
Gerrit-Owner: Pooja Nilangekar <[email protected]>
Gerrit-Reviewer: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Bikramjeet Vig <[email protected]>
Gerrit-Reviewer: Fredy Wijaya <[email protected]>
Gerrit-Reviewer: Pooja Nilangekar <[email protected]>
Gerrit-Comment-Date: Mon, 23 Jul 2018 06:45:26 +0000
Gerrit-HasComments: Yes

Reply via email to