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 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/10908/4/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/4/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java@72 PS4, Line 72: ((SelectStmt) stmt).getTableRefs(); This only gets you fromClause_'s TableRefs. For example something like alter view v1 as select * from test1 where a > (select count(*) from v1); would pass this and then the view analysis causes stack overflow. (add tests for non-from clause cases) http://gerrit.cloudera.org:8080/#/c/10908/4/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java@75 PS4, Line 75: InlineViewRef fromViewRef = (InlineViewRef) fromTblRef; While I see the intention here, a cleaner way for this is to do something like what QueryStmt#collectTableRefs() does. While the name says "TableRefs", it also expands the InlineViewRefs to populate the actual base TableRefs. -- 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: 4 Gerrit-Owner: Pooja Nilangekar <pooja.nilange...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Pooja Nilangekar <pooja.nilange...@cloudera.com> Gerrit-Comment-Date: Sat, 14 Jul 2018 22:42:02 +0000 Gerrit-HasComments: Yes
