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 6: (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@75 PS4, Line 75: InlineViewRef fromViewRef = (InlineViewRef) fromTblRef; > That was my initial approach. However, I faced the issue that while getting right, we ideally need to implement something like collectInlineViewRefs() in all the subclasses but that is probably not worth the effort since it is not used anywhere else. http://gerrit.cloudera.org:8080/#/c/10908/6/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/6/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java@69 PS6, Line 69: while (!subQueries.isEmpty()) { : QueryStmt stmt = subQueries.pop(); : if (stmt instanceof SelectStmt) { : List<TableRef> fromTblRefs = ((SelectStmt) stmt).getTableRefs(); : for (TableRef fromTblRef : fromTblRefs) { : if (fromTblRef instanceof InlineViewRef) { : InlineViewRef fromViewRef = (InlineViewRef) fromTblRef; : if (fromViewRef.getView() == ((InlineViewRef) tblRef).getView()) { : throw new AnalysisException(String.format( : "Self-reference not allowed on view: %s", tblRef.toSql())); : } else { : subQueries.push(fromViewRef.getViewStmt()); : } : } : } : QueryStmt whereStmt = ((SelectStmt) stmt).getWhereQueryStmt(); : if (whereStmt != null) { : subQueries.add(whereStmt); : } : } else if (stmt instanceof UnionStmt) { : List<UnionOperand> unionOperands = ((UnionStmt) stmt).getOperands(); : for (UnionOperand operand : unionOperands) { : subQueries.push(operand.getQueryStmt()); : } : } else { : throw new AnalysisException("Subqueries not supported for " + : stmt.getClass().getSimpleName() + " statements"); : } : How about factoring this out into a method in QueryStmt and have helper methods in SelectStmt and UnionStmt to collect each of their own view refs? That way you could do something like if (viewDefStmt_.collectInlineViewRefs().contains(tblRef.getView())... -- 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: 6 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: Fri, 20 Jul 2018 17:28:36 +0000 Gerrit-HasComments: Yes
