You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Fredy Wijaya (Code Review)" <ge...@cloudera.org> on 2019/04/08 21:19:14 UTC
[Impala-ASF-CR] IMPALA-6718: Add support for column-level permissions on views
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12959
Change subject: IMPALA-6718: Add support for column-level permissions on views
......................................................................
IMPALA-6718: Add support for column-level permissions on views
This patch adds support for column-level permissions on views. The
following statements are now supported.
GRANT select(col) ON db.my_view TO ROLE my_role -- Sentry only
REVOKE select(col) ON db.my_view FROM ROLE my_role -- Sentry only
GRANT select(col) ON db.my_view TO USER my_user -- Ranger only
REVOKE select(col) ON db.my_view FROM USER my_user -- Ranger only
GRANT select(col) ON db.my_view TO GROUP my_group -- Ranger only
REVOKE select(col) ON db.my_view FROM GROUP my_group -- Ranger only
Testing:
- Updated AuthorizationStmtTest to with new test cases
- Ran all FE tests
- Ran all E2E authorization tests
Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
---
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
3 files changed, 30 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/12959/2
--
To view, visit http://gerrit.cloudera.org:8080/12959
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
Gerrit-Change-Number: 12959
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
[Impala-ASF-CR] IMPALA-6718: Add support for column-level permissions on views
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12959 )
Change subject: IMPALA-6718: Add support for column-level permissions on views
......................................................................
Patch Set 7:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4023/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12959
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
Gerrit-Change-Number: 12959
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 15 Apr 2019 22:47:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6718: Add support for column-level permissions on views
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/12959 )
Change subject: IMPALA-6718: Add support for column-level permissions on views
......................................................................
IMPALA-6718: Add support for column-level permissions on views
This patch adds support for column-level permissions on views. This
behavior is compatible with Hive column-level permissons on views. The
following statements are now supported.
GRANT select(col) ON db.my_view TO ROLE my_role -- Sentry only
REVOKE select(col) ON db.my_view FROM ROLE my_role -- Sentry only
GRANT select(col) ON db.my_view TO USER my_user -- Ranger only
REVOKE select(col) ON db.my_view FROM USER my_user -- Ranger only
GRANT select(col) ON db.my_view TO GROUP my_group -- Ranger only
REVOKE select(col) ON db.my_view FROM GROUP my_group -- Ranger only
Testing:
- Updated AuthorizationStmtTest to with new test cases
- Ran all FE tests
- Ran all E2E authorization tests
Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
---
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
4 files changed, 82 insertions(+), 30 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/12959/6
--
To view, visit http://gerrit.cloudera.org:8080/12959
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
Gerrit-Change-Number: 12959
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[Impala-ASF-CR] IMPALA-6718: Add support for column-level permissions on views
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/12959 )
Change subject: IMPALA-6718: Add support for column-level permissions on views
......................................................................
IMPALA-6718: Add support for column-level permissions on views
This patch adds support for column-level permissions on views. The
following statements are now supported.
GRANT select(col) ON db.my_view TO ROLE my_role -- Sentry only
REVOKE select(col) ON db.my_view FROM ROLE my_role -- Sentry only
GRANT select(col) ON db.my_view TO USER my_user -- Ranger only
REVOKE select(col) ON db.my_view FROM USER my_user -- Ranger only
GRANT select(col) ON db.my_view TO GROUP my_group -- Ranger only
REVOKE select(col) ON db.my_view FROM GROUP my_group -- Ranger only
Testing:
- Updated AuthorizationStmtTest to with new test cases
- Ran all FE tests
- Ran all E2E authorization tests
Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
---
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
4 files changed, 33 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/12959/3
--
To view, visit http://gerrit.cloudera.org:8080/12959
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
Gerrit-Change-Number: 12959
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[Impala-ASF-CR] IMPALA-6718: Add support for column-level permissions on views
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12959 )
Change subject: IMPALA-6718: Add support for column-level permissions on views
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12959
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
Gerrit-Change-Number: 12959
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 15 Apr 2019 22:47:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6718: Add support for column-level permissions on views
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12959 )
Change subject: IMPALA-6718: Add support for column-level permissions on views
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12959/5/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/12959/5/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@576
PS5, Line 576: List<Expr> slotRefs = new ArrayList<>();
: expr.collectAll(Predicates.instanceOf(SlotRef.class), slotRefs);
: for (Expr e: slotRefs) {
: SlotRef slotRef = (SlotRef) e;
: analyzer_.registerPrivReq(builder -> builder
: .allOf(Privilege.SELECT)
: .onColumn(view.getDb().getName(), view.getName(),
: slotRef.getDesc().getLabel())
: .build());
: }
: }
: }
: }
:
> Consider using TreeNode#collect()?
Done
--
To view, visit http://gerrit.cloudera.org:8080/12959
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
Gerrit-Change-Number: 12959
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 15 Apr 2019 17:59:11 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6718: Add support for column-level permissions on views
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12959 )
Change subject: IMPALA-6718: Add support for column-level permissions on views
......................................................................
Patch Set 5:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12959/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12959/3//COMMIT_MSG@7
PS3, Line 7: IMPALA-6718: Add support for column-level permissions on views
> Does it make sense to call out Hive's behavior here and mention that we are
Done
http://gerrit.cloudera.org:8080/#/c/12959/3/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/12959/3/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@550
PS3, Line 550: FeView view = inlineViewRef.getView();
> Add a comment why we skip these?
Done
http://gerrit.cloudera.org:8080/#/c/12959/3/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@553
PS3, Line 553: privileges. For example:
> doesn't look right to me.. (add some test coverage)
You're right. Done
--
To view, visit http://gerrit.cloudera.org:8080/12959
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
Gerrit-Change-Number: 12959
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 15 Apr 2019 17:12:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6718: Add support for column-level permissions on views
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12959 )
Change subject: IMPALA-6718: Add support for column-level permissions on views
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2779/ : 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/12959
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
Gerrit-Change-Number: 12959
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 15 Apr 2019 17:54:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6718: Add support for column-level permissions on views
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12959 )
Change subject: IMPALA-6718: Add support for column-level permissions on views
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12959/3/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/12959/3/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@545
PS3, Line 545: private void registerViewColumnPrivileges() {
: for (TableRef tableRef: fromClause_.getTableRefs()) {
: if (!(tableRef instanceof InlineViewRef)) continue;
: InlineViewRef inlineViewRef = (InlineViewRef) tableRef;
: FeView view = inlineViewRef.getView();
: boolean isCatalogView = view != null && !view.isLocalView();
: if (!isCatalogView) continue;
: for (Expr expr : getResultExprs()) {
: if (!(expr instanceof SlotRef)) continue;
: SlotRef slotRef = (SlotRef) expr;
: analyzer_.registerPrivReq(builder -> builder
: .allOf(Privilege.SELECT)
: .onColumn(view.getDb().getName(), view.getName(),
: slotRef.getDesc().getLabel())
: .build());
: }
: }
: }
> Had a chat with Fredy offline. We were discussing if there is a more cleane
Had a chat offline. I don't think this is possible to do in InlineViewRef analysis since if we have this statement:
create view v(a, b) as select id, string_col from functional.alltypes
select a from v
The only way to know "a" is in the SelectList is in the SelectStmt analysis.
--
To view, visit http://gerrit.cloudera.org:8080/12959
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
Gerrit-Change-Number: 12959
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 11 Apr 2019 16:04:30 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6718: Add support for column-level permissions on views
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/12959 )
Change subject: IMPALA-6718: Add support for column-level permissions on views
......................................................................
IMPALA-6718: Add support for column-level permissions on views
This patch adds support for column-level permissions on views. This
behavior is compatible with Hive column-level permissons on views. The
following statements are now supported.
GRANT select(col) ON db.my_view TO ROLE my_role -- Sentry only
REVOKE select(col) ON db.my_view FROM ROLE my_role -- Sentry only
GRANT select(col) ON db.my_view TO USER my_user -- Ranger only
REVOKE select(col) ON db.my_view FROM USER my_user -- Ranger only
GRANT select(col) ON db.my_view TO GROUP my_group -- Ranger only
REVOKE select(col) ON db.my_view FROM GROUP my_group -- Ranger only
Testing:
- Updated AuthorizationStmtTest to with new test cases
- Ran all FE tests
- Ran all E2E authorization tests
Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
---
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
4 files changed, 86 insertions(+), 30 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/12959/5
--
To view, visit http://gerrit.cloudera.org:8080/12959
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
Gerrit-Change-Number: 12959
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[Impala-ASF-CR] IMPALA-6718: Add support for column-level permissions on views
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12959 )
Change subject: IMPALA-6718: Add support for column-level permissions on views
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12959/3/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/12959/3/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@545
PS3, Line 545: private void registerViewColumnPrivileges() {
: for (TableRef tableRef: fromClause_.getTableRefs()) {
: if (!(tableRef instanceof InlineViewRef)) continue;
: InlineViewRef inlineViewRef = (InlineViewRef) tableRef;
: FeView view = inlineViewRef.getView();
: boolean isCatalogView = view != null && !view.isLocalView();
: if (!isCatalogView) continue;
: for (Expr expr : getResultExprs()) {
: if (!(expr instanceof SlotRef)) continue;
: SlotRef slotRef = (SlotRef) expr;
: analyzer_.registerPrivReq(builder -> builder
: .allOf(Privilege.SELECT)
: .onColumn(view.getDb().getName(), view.getName(),
: slotRef.getDesc().getLabel())
: .build());
: }
: }
: }
Had a chat with Fredy offline. We were discussing if there is a more cleaner way to do it in the InlineViewRef analysis. We could mark the view slots as materialized and add column privileges just like how we do for table columns.
--
To view, visit http://gerrit.cloudera.org:8080/12959
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
Gerrit-Change-Number: 12959
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 09 Apr 2019 00:28:25 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6718: Add support for column-level permissions on views
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12959 )
Change subject: IMPALA-6718: Add support for column-level permissions on views
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2780/ : 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/12959
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
Gerrit-Change-Number: 12959
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 15 Apr 2019 18:33:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6718: Add support for column-level permissions on views
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12959 )
Change subject: IMPALA-6718: Add support for column-level permissions on views
......................................................................
Patch Set 3:
(3 comments)
sorry for the delay here.
http://gerrit.cloudera.org:8080/#/c/12959/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12959/3//COMMIT_MSG@7
PS3, Line 7: IMPALA-6718: Add support for column-level permissions on views
Does it make sense to call out Hive's behavior here and mention that we are compatible with it?
Specially the part about view's column level privs here mean that they only apply to view's columns and not base table's columns that view's column map to. Even if someone revokes base table column's privs, someone with view's col access can still access them. (feel free to add some test coverage for it if you think that helps).
http://gerrit.cloudera.org:8080/#/c/12959/3/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/12959/3/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@550
PS3, Line 550: boolean isCatalogView = view != null && !view.isLocalView();
Add a comment why we skip these?
http://gerrit.cloudera.org:8080/#/c/12959/3/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@553
PS3, Line 553: if (!(expr instanceof SlotRef)) continue;
doesn't look right to me.. (add some test coverage)
how about something like select 2 * v.id from v (doesn't always need to be a slotref)
--
To view, visit http://gerrit.cloudera.org:8080/12959
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
Gerrit-Change-Number: 12959
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 15 Apr 2019 01:51:01 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6718: Add support for column-level permissions on views
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12959 )
Change subject: IMPALA-6718: Add support for column-level permissions on views
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12959/5/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/12959/5/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@576
PS5, Line 576: Stack<Expr> exprs = new Stack<>();
: exprs.addAll(getResultExprs());
: while (!exprs.isEmpty()) {
: Expr expr = exprs.pop();
: if (!expr.getChildren().isEmpty()) {
: exprs.addAll(expr.getChildren());
: } else if (expr instanceof SlotRef) {
: SlotRef slotRef = (SlotRef) expr;
: analyzer_.registerPrivReq(builder -> builder
: .allOf(Privilege.SELECT)
: .onColumn(view.getDb().getName(), view.getName(),
: slotRef.getDesc().getLabel())
: .build());
: }
Consider using TreeNode#collect()?
--
To view, visit http://gerrit.cloudera.org:8080/12959
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
Gerrit-Change-Number: 12959
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 15 Apr 2019 17:40:30 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6718: Add support for column-level permissions on views
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12959 )
Change subject: IMPALA-6718: Add support for column-level permissions on views
......................................................................
Patch Set 7: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/12959
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
Gerrit-Change-Number: 12959
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 16 Apr 2019 03:49:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6718: Add support for column-level permissions on views
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12959 )
Change subject: IMPALA-6718: Add support for column-level permissions on views
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2684/ : 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/12959
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
Gerrit-Change-Number: 12959
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 08 Apr 2019 21:46:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6718: Add support for column-level permissions on views
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12959 )
Change subject: IMPALA-6718: Add support for column-level permissions on views
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12959
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
Gerrit-Change-Number: 12959
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 15 Apr 2019 22:00:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6718: Add support for column-level permissions on views
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12959 )
Change subject: IMPALA-6718: Add support for column-level permissions on views
......................................................................
IMPALA-6718: Add support for column-level permissions on views
This patch adds support for column-level permissions on views. This
behavior is compatible with Hive column-level permissons on views. The
following statements are now supported.
GRANT select(col) ON db.my_view TO ROLE my_role -- Sentry only
REVOKE select(col) ON db.my_view FROM ROLE my_role -- Sentry only
GRANT select(col) ON db.my_view TO USER my_user -- Ranger only
REVOKE select(col) ON db.my_view FROM USER my_user -- Ranger only
GRANT select(col) ON db.my_view TO GROUP my_group -- Ranger only
REVOKE select(col) ON db.my_view FROM GROUP my_group -- Ranger only
Testing:
- Updated AuthorizationStmtTest to with new test cases
- Ran all FE tests
- Ran all E2E authorization tests
Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
Reviewed-on: http://gerrit.cloudera.org:8080/12959
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
4 files changed, 82 insertions(+), 30 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/12959
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
Gerrit-Change-Number: 12959
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[Impala-ASF-CR] IMPALA-6718: Add support for column-level permissions on views
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12959 )
Change subject: IMPALA-6718: Add support for column-level permissions on views
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2688/ : 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/12959
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If81e683212cba22cc0fa5fc091ec3c799fa33e14
Gerrit-Change-Number: 12959
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 08 Apr 2019 22:43:23 +0000
Gerrit-HasComments: No