[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10908 ) Change subject: IMPALA-7209: Disallow self referencing in ALTER VIEW statements .. IMPALA-7209: Disallow self referencing in ALTER VIEW statements Previously, ALTER VIEW queries did not carry out reference checks in the analysis phase. This allowed the DDL operation to succeed but subsequent queries to the view threw StackOverflowError because the catalog was unable to resolve the reference. With this change, the AlterViewStmt checks for direct and in-direct self references before altering a view. Testing: Added tests to AnalyzeDDLTest to verify it. Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 Reviewed-on: http://gerrit.cloudera.org:8080/10908 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java 5 files changed, 123 insertions(+), 1 deletion(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- 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: merged Gerrit-Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 Gerrit-Change-Number: 10908 Gerrit-PatchSet: 15 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Impala Public Jenkins 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 14: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2880/ DRY_RUN=false -- 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: 14 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Mon, 30 Jul 2018 22:50:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Impala Public Jenkins 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 13: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/104/ : Initial code review checks failed. See linked job for details on the failure. -- 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: 13 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Mon, 30 Jul 2018 22:37:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Impala Public Jenkins 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 14: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2878/ DRY_RUN=false -- 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: 14 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Mon, 30 Jul 2018 22:14:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Impala Public Jenkins 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 14: Code-Review+2 -- 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: 14 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Mon, 30 Jul 2018 22:14:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Impala Public Jenkins 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 13: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/104/ Running initial code review checks. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- 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: 13 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Mon, 30 Jul 2018 17:46:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Pooja Nilangekar has uploaded a new patch set (#13). ( http://gerrit.cloudera.org:8080/10908 ) Change subject: IMPALA-7209: Disallow self referencing in ALTER VIEW statements .. IMPALA-7209: Disallow self referencing in ALTER VIEW statements Previously, ALTER VIEW queries did not carry out reference checks in the analysis phase. This allowed the DDL operation to succeed but subsequent queries to the view threw StackOverflowError because the catalog was unable to resolve the reference. With this change, the AlterViewStmt checks for direct and in-direct self references before altering a view. Testing: Added tests to AnalyzeDDLTest to verify it. Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 --- M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java 5 files changed, 123 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/10908/13 -- 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: newpatchset Gerrit-Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 Gerrit-Change-Number: 10908 Gerrit-PatchSet: 13 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Pooja Nilangekar 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 13: (1 comment) http://gerrit.cloudera.org:8080/#/c/10908/12/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/12/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@147 PS12, Line 147: /** > nit: Use /* */ style comments for methods Done -- 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: 13 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Mon, 30 Jul 2018 17:43:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Impala Public Jenkins 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 12: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/99/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 12 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Mon, 30 Jul 2018 17:37:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
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 12: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/10908/12/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/12/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@147 PS12, Line 147: // Returns the QueryStmt present in the whereClause_ if present, null otherwise. nit: Use /* */ style comments for methods -- 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: 12 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Mon, 30 Jul 2018 17:20:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Pooja Nilangekar 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 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/10908/11/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java File fe/src/main/java/org/apache/impala/analysis/UnionStmt.java: http://gerrit.cloudera.org:8080/#/c/10908/11/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java@557 PS11, Line 557: super.collectInlineViews(inlineViews); > super.collectInlineViews() Done -- 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: 12 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Mon, 30 Jul 2018 17:03:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Impala Public Jenkins 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 12: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/99/ Running initial code review checks. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- 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: 12 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Mon, 30 Jul 2018 17:03:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Pooja Nilangekar has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/10908 ) Change subject: IMPALA-7209: Disallow self referencing in ALTER VIEW statements .. IMPALA-7209: Disallow self referencing in ALTER VIEW statements Previously, ALTER VIEW queries did not carry out reference checks in the analysis phase. This allowed the DDL operation to succeed but subsequent queries to the view threw StackOverflowError because the catalog was unable to resolve the reference. With this change, the AlterViewStmt checks for direct and in-direct self references before altering a view. Testing: Added tests to AnalyzeDDLTest to verify it. Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 --- M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java 5 files changed, 121 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/10908/12 -- 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: newpatchset Gerrit-Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 Gerrit-Change-Number: 10908 Gerrit-PatchSet: 12 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
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 11:
(1 comment)
Can +2 once you fix this remaining bug.
http://gerrit.cloudera.org:8080/#/c/10908/11/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
File fe/src/main/java/org/apache/impala/analysis/UnionStmt.java:
http://gerrit.cloudera.org:8080/#/c/10908/11/fe/src/main/java/org/apache/impala/analysis/UnionStmt.java@557
PS11, Line 557: for (UnionOperand operand : operands_) {
super.collectInlineViews()
Add a test for this too. (view self ref from withClause_ in a UnionStmt).
--
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: 11
Gerrit-Owner: Pooja Nilangekar
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Pooja Nilangekar
Gerrit-Comment-Date: Mon, 30 Jul 2018 01:21:50 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Impala Public Jenkins 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 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/95/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 11 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Sat, 28 Jul 2018 01:01:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Impala Public Jenkins 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 11: Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/95/ Running initial code review checks. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- 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: 11 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Sat, 28 Jul 2018 00:21:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Pooja Nilangekar has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/10908 ) Change subject: IMPALA-7209: Disallow self referencing in ALTER VIEW statements .. IMPALA-7209: Disallow self referencing in ALTER VIEW statements Previously, ALTER VIEW queries did not carry out reference checks in the analysis phase. This allowed the DDL operation to succeed but subsequent queries to the view threw StackOverflowError because the catalog was unable to resolve the reference. With this change, the AlterViewStmt checks for direct and in-direct self references before altering a view. Testing: Added tests to AnalyzeDDLTest to verify it. Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 --- M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java 5 files changed, 108 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/10908/11 -- 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: newpatchset Gerrit-Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 Gerrit-Change-Number: 10908 Gerrit-PatchSet: 11 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
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 10:
(3 comments)
Sorry for the delay, I was busy this week.
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
> I tried the changes discussed offline. However, the InlineViewRef objects a
Makes sense. So can we call this collectIlineViews(..views) or something since
we are not collecting ViewRefs anyway?
http://gerrit.cloudera.org:8080/#/c/10908/8/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/8/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@138
PS8, Line 138: public abstract void collectInlineViewRefs(Set
inlineViewRefs);
> Done.
Let's keep both the versions for completeness. Looks like you replaced the
earlier one.
http://gerrit.cloudera.org:8080/#/c/10908/10/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/10/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@1092
PS10, Line 1092: if (withClause_ != null) {
: List withClauseViews =
withClause_.getViews();
: for (FeView withView : withClauseViews) {
: inlineViewRefs.add(withView);
:
withView.getQueryStmt().collectInlineViewRefs(inlineViewRefs);
: }
: }
Should this be moved to the QueryStmt class? See implementation of
collectTableRefs() for example.
--
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: 10
Gerrit-Owner: Pooja Nilangekar
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Pooja Nilangekar
Gerrit-Comment-Date: Sat, 28 Jul 2018 00:06:45 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Impala Public Jenkins 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 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/27/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 10 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Tue, 24 Jul 2018 01:51:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Pooja Nilangekar has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/10908 ) Change subject: IMPALA-7209: Disallow self referencing in ALTER VIEW statements .. IMPALA-7209: Disallow self referencing in ALTER VIEW statements Previously, ALTER VIEW queries did not carry out reference checks in the analysis phase. This allowed the DDL operation to succeed but subsequent queries to the view threw StackOverflowError because the catalog was unable to resolve the reference. With this change, the AlterViewStmt checks for direct and in-direct self references before altering a view. Testing: Added tests to AnalyzeDDLTest to verify it. Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 --- M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java 5 files changed, 101 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/10908/10 -- 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: newpatchset Gerrit-Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 Gerrit-Change-Number: 10908 Gerrit-PatchSet: 10 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Pooja Nilangekar
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Pooja Nilangekar 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 10: (3 comments) 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 > As discussed offline, using InlineViewRef makes it more consistent with col I tried the changes discussed offline. However, the InlineViewRef objects are not equal (their corresponding SQL strings match but the hash of the objects are different). On the other hand, equality check of FeView only compares the QueryStmt and the ColRefs which are equal. http://gerrit.cloudera.org:8080/#/c/10908/8/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/8/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@138 PS8, Line 138: public abstract void collectInlineViewRefs(Set inlineViewRefs); > Shouldn't we also collect view refs from withClause_? I see you added a tes Done. I have also changed the test. 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@148 PS7, Line 148: getWhereSubQuery > I think is is ok since it conveys the intention better than getWhereQuerySt Done -- 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: 10 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Tue, 24 Jul 2018 01:14:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
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 8: (3 comments) 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 > I had no semantic reason, I was only trying to follow the convention of col As discussed offline, using InlineViewRef makes it more consistent with collectTableRefs() http://gerrit.cloudera.org:8080/#/c/10908/8/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/8/fe/src/main/java/org/apache/impala/analysis/QueryStmt.java@138 PS8, Line 138: public abstract void collectInlineViewRefs(Set inlineViewRefs); Shouldn't we also collect view refs from withClause_? I see you added a test for it but something like the following would fail. Basically with clause's subqueries not referenced in from clause of the select stmt. (I couldn't think of a better example :D). alter view v2 as with temp(col_a) as (select * from v2) select * from t2; The reason the test case you added works is because the view referenced is also indirectly referenced in the fromClause_ of the SelectStmt. 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@148 PS7, Line 148: getWhereQueryStm > I have changed it to private. I initially named it getWhereSubQueryStmt but I think is is ok since it conveys the intention better than getWhereQueryStmt(). -- 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: 8 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Mon, 23 Jul 2018 23:57:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Pooja Nilangekar 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 8:
(7 comments)
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.Set;
> Remove
Done
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.catalog.FeTable;
> Remove
Done
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 directly?
I had no semantic reason, I was only trying to follow the convention of
collectTableRefs. In that function the TableRefs are collected in SQL clause
order.
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_ if
present, null otherwise.
> ...Null otherwise...
Done
http://gerrit.cloudera.org:8080/#/c/10908/7/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@148
PS7, Line 148: getWhereQueryStm
> I think getWhereSubQueryStmt() makes more sense. Also make it private?
I have changed it to private. I initially named it getWhereSubQueryStmt but
then noticed that there is another class called SubQuery. So I thought it might
be misleading. Do you think it would be okay to change it?
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 inlineViewRefs)
{
> nit: Mention that currently only from/where subqueries are supported. Other
Done
http://gerrit.cloudera.org:8080/#/c/10908/7/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@1077
PS7, Line 1077: // Impala currently supports sub queries only in FROM &
WHERE clauses. Hence, this
> Preconditions.checkNotNull(inlineViewRefs);
Done
--
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: 8
Gerrit-Owner: Pooja Nilangekar
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Pooja Nilangekar
Gerrit-Comment-Date: Mon, 23 Jul 2018 18:07:40 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Pooja Nilangekar has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/10908 ) Change subject: IMPALA-7209: Disallow self referencing in ALTER VIEW statements .. IMPALA-7209: Disallow self referencing in ALTER VIEW statements Previously, ALTER VIEW queries did not carry out reference checks in the analysis phase. This allowed the DDL operation to succeed but subsequent queries to the view threw StackOverflowError because the catalog was unable to resolve the reference. With this change, the AlterViewStmt checks for direct and in-direct self references before altering a view. Testing: Added tests to AnalyzeDDLTest to verify it. Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 --- M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java 5 files changed, 94 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/10908/8 -- 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: newpatchset Gerrit-Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 Gerrit-Change-Number: 10908 Gerrit-PatchSet: 8 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Pooja Nilangekar
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
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 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 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 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
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Pooja Nilangekar
Gerrit-Comment-Date: Mon, 23 Jul 2018 06:45:26 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Pooja Nilangekar 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:
(1 comment)
I am sorry I had to rebase this change. I couldn't get impalad to start due to
IMPALA-7316.
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: if (inlineViewRefs.contains(((InlineViewRef) tblRef).getView())) {
: throw new AnalysisException(
: String.format("Self-reference not allowed on view: %s",
tblRef.toSql()));
: }
:
: createColumnAndViewDefs(analyzer);
: if (BackendConfig.INSTANCE.getComputeLineage() ||
RuntimeEnv.INSTANCE.isTestEnv()) {
: computeLineageGraph(analyzer);
: }
: }
:
: @Override
: public String toSql() {
: StringBuilder sb = new StringBuilder();
: sb.append("ALTER VIEW ");
: if (tableName_.getDb() != null) {
: sb.append(tableName_.getDb() + ".");
: }
: sb.append(tableName_.getTbl());
: if (columnDefs_ != null) sb.append("(" + getColumnNames() +
")");
: sb.append(" AS " + viewDefStmt_.toSql());
: return sb.toString();
: }
: }
:
:
:
:
:
> How about factoring this out into a method in QueryStmt and have helper met
Done
--
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
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Pooja Nilangekar
Gerrit-Comment-Date: Sat, 21 Jul 2018 01:30:37 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Pooja Nilangekar has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/10908 ) Change subject: IMPALA-7209: Disallow self referencing in ALTER VIEW statements .. IMPALA-7209: Disallow self referencing in ALTER VIEW statements Previously, ALTER VIEW queries did not carry out reference checks in the analysis phase. This allowed the DDL operation to succeed but subsequent queries to the view threw StackOverflowError because the catalog was unable to resolve the reference. With this change, the AlterViewStmt checks for direct and in-direct self references before altering a view. Testing: Added tests to AnalyzeDDLTest to verify it. Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 --- M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/QueryStmt.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java 5 files changed, 93 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/10908/7 -- 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: newpatchset Gerrit-Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 Gerrit-Change-Number: 10908 Gerrit-PatchSet: 7 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Pooja Nilangekar
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
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 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 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
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Pooja Nilangekar
Gerrit-Comment-Date: Fri, 20 Jul 2018 17:28:36 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Pooja Nilangekar has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/10908 ) Change subject: IMPALA-7209: Disallow self referencing in ALTER VIEW statements .. IMPALA-7209: Disallow self referencing in ALTER VIEW statements Previously, ALTER VIEW queries did not carry out reference checks in the analysis phase. This allowed the DDL operation to succeed but subsequent queries to the view threw StackOverflowError because the catalog was unable to resolve the reference. With this change, the AlterViewStmt checks for direct and in-direct self references before altering a view. Testing: Added tests to AnalyzeDDLTest to verify it. Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 --- M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java 3 files changed, 85 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/10908/6 -- 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: newpatchset Gerrit-Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 Gerrit-Change-Number: 10908 Gerrit-PatchSet: 6 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Pooja Nilangekar
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Pooja Nilangekar 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@72 PS4, Line 72: ((SelectStmt) stmt).getTableRefs(); > This only gets you fromClause_'s TableRefs. For example something like Done. I have added test cases for where clause and with clause. Other classes like "GROUP BY", "ORDER BY" and "HAVING" don't support sub-queries. 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 l That was my initial approach. However, I faced the issue that while getting the TableRefs from the viewDefStmt_, the function resolves it to the underlying table definition. Since at this point, the view definition is still not altered. Yet, the SQL statement contains references to the view itself. So this approach ensures that we get all the view references that need to be resolved instead of getting just the eventual base table references. -- 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 Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Tue, 17 Jul 2018 20:56:08 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
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 Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Sat, 14 Jul 2018 22:42:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Fredy Wijaya 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: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/10908/2/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/2/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java@91 PS2, Line 91: stmt.getClass().getSimpleName() + " statements"); : } > Personally, I think the exception make sense when some external event cause Thanks for the explanation! -- 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 Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Pooja Nilangekar Gerrit-Comment-Date: Wed, 11 Jul 2018 19:02:39 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Pooja Nilangekar has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/10908 ) Change subject: IMPALA-7209: Disallow self referencing in ALTER VIEW statements .. IMPALA-7209: Disallow self referencing in ALTER VIEW statements Previously, ALTER VIEW queries did not carry out reference checks in the analysis phase. This allowed the DDL operation to succeed but subsequent queries to the view threw StackOverflowError because the catalog was unable to resolve the reference. With this change, the AlterViewStmt checks for direct and in-direct self references before altering a view. Testing: Added tests to AnalyzeDDLTest to verify it. Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 --- M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java 2 files changed, 58 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/10908/4 -- 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: newpatchset Gerrit-Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 Gerrit-Change-Number: 10908 Gerrit-PatchSet: 4 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Pooja Nilangekar
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Fredy Wijaya 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 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/10908/2/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/2/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java@65
PS2, Line 65: TableRef tblRef = new TableRef(tableName_.toPath(), null);
: tblRef = analyzer.resolveTableRef(tblRef);
nit: can be simplified to TableRef tblRef = analyzer.resolveTableRef(new
TableRef(tableName_.toPath(), null));
http://gerrit.cloudera.org:8080/#/c/10908/2/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java@79
PS2, Line 79: Self reference
nit: Self-reference
http://gerrit.cloudera.org:8080/#/c/10908/2/fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java@91
PS2, Line 91: throw new AnalysisException("Subqueries not supported for " +
: stmt.getClass().getSimpleName() + " statements");
I'm a bit torn whether we should throw an AnalysisException or have a
Preconditions check since this condition should never happen given that the
parser doesn't support this syntax. Let me know what you think?
--
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: 2
Gerrit-Owner: Pooja Nilangekar
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Pooja Nilangekar
Gerrit-Comment-Date: Wed, 11 Jul 2018 18:36:05 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7209: Disallow self referencing in ALTER VIEW statements
Pooja Nilangekar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10908 Change subject: IMPALA-7209: Disallow self referencing in ALTER VIEW statements .. IMPALA-7209: Disallow self referencing in ALTER VIEW statements Previously, ALTER VIEW queries did not carry out reference checks in the analysis phase. This allowed the DDL operation to succeed but subsequent queries to the view threw StackOverflowError because the catalog was unable to resolve the reference. With this change, the AlterViewStmt checks for direct and in-direct self references before altering a view. Testing: Added tests to AnalyzeDDLTest to verify it. Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 --- M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java 2 files changed, 59 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/10908/2 -- 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: newchange Gerrit-Change-Id: I17c231c9d74d9d411463a408b086eb874090b9b7 Gerrit-Change-Number: 10908 Gerrit-PatchSet: 2 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Pooja Nilangekar
