subject:"\[Impala\-ASF\-CR\] IMPALA\-7074\: Update OWNER privilege on CREATE, DROP, and SET OWNER"

Adam Holley has uploaded a new patch set (#42). ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..

IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

This patch adds calls to automatically create or remove owner
privileges in the catalog based on the statement.  This is similar to
the existing pattern where after privileges are granted in Sentry,
they are created in the catalog directly instead of pulled from
Sentry.

When object ownership is enabled:
CREATE DATABASE will grant the user OWNER privileges to that database.
ALTER DATABASE SET OWNER will transfer the OWNER privileges to the
new owner.
DROP DATABASE will revoke the OWNER privileges from the owner.
This will apply to DATABASE, TABLE, and VIEW.

Example:
If ownership is enabled, when a table is created, the creator is the
owner, and Sentry will create owner privileges for the created table so
the user can continue working with it without waiting for Sentry
refresh.  Inserts will be available immediately.

Testing:
- Created new custom cluster tests for object ownership

Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
---
M bin/create-test-configuration.sh
M bin/impala-config.sh
M common/thrift/JniCatalog.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/resources/mysql-hive-site.xml.template
M fe/src/test/resources/postgresql-hive-site.xml.template
M fe/src/test/resources/sentry-site.xml.template
A fe/src/test/resources/sentry-site_no_oo.xml.template
A fe/src/test/resources/sentry-site_oo.xml.template
A fe/src/test/resources/sentry-site_oo_nogrant.xml.template
M testdata/bin/run-sentry-service.sh
M tests/authorization/test_grant_revoke.py
A tests/authorization/test_owner_privileges.py
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_test_suite.py
34 files changed, 1,365 insertions(+), 154 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/11314/42
--
To view, visit http://gerrit.cloudera.org:8080/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 42
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-13 Thread Vuk Ercegovac (Code Review)

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 41: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 41
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 14 Sep 2018 01:12:08 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-13 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 41:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11314/41/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java:

http://gerrit.cloudera.org:8080/#/c/11314/41/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2004
PS41, Line 2004: .error(accessError(true, "functional.alltypes"), 
onDatabase(true, "functional",
line too long (91 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 41
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 14 Sep 2018 00:59:38 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-13 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 40: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 40
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 13 Sep 2018 23:08:21 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-13 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 40:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11314/40/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java:

http://gerrit.cloudera.org:8080/#/c/11314/40/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2003
PS40, Line 2003: .error(accessError(true, "functional.alltypes"), 
onDatabase(true, "functional",
line too long (91 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 40
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 13 Sep 2018 22:40:08 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-13 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 41:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/671/ : 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/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 41
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 13 Sep 2018 21:58:04 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has uploaded a new patch set (#41). ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..

IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

This patch adds calls to automatically create or remove owner
privileges in the catalog based on the statement.  This is similar to
the existing pattern where after privileges are granted in Sentry,
they are created in the catalog directly instead of pulled from
Sentry.

When object ownership is enabled:
CREATE DATABASE will grant the user OWNER privileges to that database.
ALTER DATABASE SET OWNER will transfer the OWNER privileges to the
new owner.
DROP DATABASE will revoke the OWNER privileges from the owner.
This will apply to DATABASE, TABLE, and VIEW.

Example:
If ownership is enabled, when a table is created, the creator is the
owner, and Sentry will create owner privileges for the created table so
the user can continue working with it without waiting for Sentry
refresh.  Inserts will be available immediately.

Testing:
- Created new custom cluster tests for object ownership

Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
---
M bin/create-test-configuration.sh
M bin/impala-config.sh
M common/thrift/JniCatalog.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/resources/mysql-hive-site.xml.template
M fe/src/test/resources/postgresql-hive-site.xml.template
M fe/src/test/resources/sentry-site.xml.template
A fe/src/test/resources/sentry-site_no_oo.xml.template
A fe/src/test/resources/sentry-site_oo.xml.template
A fe/src/test/resources/sentry-site_oo_nogrant.xml.template
M testdata/bin/run-sentry-service.sh
M tests/authorization/test_grant_revoke.py
A tests/authorization/test_owner_privileges.py
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_test_suite.py
34 files changed, 1,370 insertions(+), 156 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/11314/41
--
To view, visit http://gerrit.cloudera.org:8080/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 41
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 40:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/11314/37/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/37/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2918
PS37, Line 2918:   
response.result.addToUpdated_catalog_objects(cPrivilege.toTCatalogObject());
> I'm somewhat concerned that you modify owner (which is a shared object) wit
Done


http://gerrit.cloudera.org:8080/#/c/11314/38/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/38/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2903
PS38, Line 2903:
> adding
Done


http://gerrit.cloudera.org:8080/#/c/11314/38/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2904
PS38, Line 2904: filter.setPrincipal_id(owner.getId());
> it also add a user if specifying a user and that user does not exist.
Done


http://gerrit.cloudera.org:8080/#/c/11314/40/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/40/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1341
PS40, Line 1341: updateOwnerPrivileges
> if this is *not* done for drop, what is the harm in waiting for the next up
If we don't drop and a different user creates the object with the same name, 
the old user will have privileges.


http://gerrit.cloudera.org:8080/#/c/11314/40/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1493
PS40, Line 1493:   if (table.getMetaStoreTable() != null) {
> same question here (why do this for drop).
Same reason.  Don't want to leave ghost privileges around in case a new object 
with the same name is created.


http://gerrit.cloudera.org:8080/#/c/11314/40/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java:

http://gerrit.cloudera.org:8080/#/c/11314/40/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2218
PS40, Line 2218:   authzCatalog_.addRole("foo_owner");
> confused by this one... after this line, "foo_owner" role exists so the sub
This role was added so that the tests will pass.  The test for when roles do 
not exist result in AnalysisException and didn't add.  Next patch will include.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 40
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 13 Sep 2018 21:13:17 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-13 Thread Vuk Ercegovac (Code Review)

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 40:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/11314/38/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/38/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2903
PS38, Line 2903:
adding


http://gerrit.cloudera.org:8080/#/c/11314/38/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2904
PS38, Line 2904: filter.setPrincipal_id(owner.getId());
it also add a user if specifying a user and that user does not exist.


http://gerrit.cloudera.org:8080/#/c/11314/40/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/40/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1341
PS40, Line 1341: updateOwnerPrivileges
if this is *not* done for drop, what is the harm in waiting for the next update 
from Sentry?


http://gerrit.cloudera.org:8080/#/c/11314/40/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1493
PS40, Line 1493:   if (table.getMetaStoreTable() != null) {
same question here (why do this for drop).


http://gerrit.cloudera.org:8080/#/c/11314/40/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java:

http://gerrit.cloudera.org:8080/#/c/11314/40/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2218
PS40, Line 2218:   authzCatalog_.addRole("foo_owner");
confused by this one... after this line, "foo_owner" role exists so the 
subsequent tests do not test the missing role case. Is the comment wrong or is 
something else off?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 40
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 13 Sep 2018 20:31:59 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-13 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 40:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/669/ : 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/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 40
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 13 Sep 2018 20:03:01 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-13 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 40:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3157/ 
DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 40
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 13 Sep 2018 19:19:37 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has uploaded a new patch set (#40). ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..

IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

This patch adds calls to automatically create or remove owner
privileges in the catalog based on the statement.  This is similar to
the existing pattern where after privileges are granted in Sentry,
they are created in the catalog directly instead of pulled from
Sentry.

When object ownership is enabled:
CREATE DATABASE will grant the user OWNER privileges to that database.
ALTER DATABASE SET OWNER will transfer the OWNER privileges to the
new owner.
DROP DATABASE will revoke the OWNER privileges from the owner.
This will apply to DATABASE, TABLE, and VIEW.

Example:
If ownership is enabled, when a table is created, the creator is the
owner, and Sentry will create owner privileges for the created table so
the user can continue working with it without waiting for Sentry
refresh.  Inserts will be available immediately.

Testing:
- Created new custom cluster tests for object ownership

Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
---
M bin/create-test-configuration.sh
M bin/impala-config.sh
M common/thrift/JniCatalog.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/resources/mysql-hive-site.xml.template
M fe/src/test/resources/postgresql-hive-site.xml.template
M fe/src/test/resources/sentry-site.xml.template
A fe/src/test/resources/sentry-site_no_oo.xml.template
A fe/src/test/resources/sentry-site_oo.xml.template
A fe/src/test/resources/sentry-site_oo_nogrant.xml.template
M testdata/bin/run-sentry-service.sh
M tests/authorization/test_grant_revoke.py
A tests/authorization/test_owner_privileges.py
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_test_suite.py
34 files changed, 1,349 insertions(+), 156 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/11314/40
--
To view, visit http://gerrit.cloudera.org:8080/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 40
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-13 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 39: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 39
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 13 Sep 2018 11:24:09 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-13 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 39:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/659/ : 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/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 39
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 13 Sep 2018 08:13:19 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-13 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 39:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3152/ 
DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 39
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 13 Sep 2018 07:40:18 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has uploaded a new patch set (#39). ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..

IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

This patch adds calls to automatically create or remove owner
privileges in the catalog based on the statement.  This is similar to
the existing pattern where after privileges are granted in Sentry,
they are created in the catalog directly instead of pulled from
Sentry.

When object ownership is enabled:
CREATE DATABASE will grant the user OWNER privileges to that database.
ALTER DATABASE SET OWNER will transfer the OWNER privileges to the
new owner.
DROP DATABASE will revoke the OWNER privileges from the owner.
This will apply to DATABASE, TABLE, and VIEW.

Example:
If ownership is enabled, when a table is created, the creator is the
owner, and Sentry will create owner privileges for the created table so
the user can continue working with it without waiting for Sentry
refresh.  Inserts will be available immediately.

Testing:
- Created new custom cluster tests for object ownership

Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
---
M bin/create-test-configuration.sh
M bin/impala-config.sh
M common/thrift/JniCatalog.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/resources/mysql-hive-site.xml.template
M fe/src/test/resources/postgresql-hive-site.xml.template
M fe/src/test/resources/sentry-site.xml.template
A fe/src/test/resources/sentry-site_no_oo.xml.template
A fe/src/test/resources/sentry-site_oo.xml.template
A fe/src/test/resources/sentry-site_oo_nogrant.xml.template
M testdata/bin/run-sentry-service.sh
M tests/authorization/test_grant_revoke.py
A tests/authorization/test_owner_privileges.py
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_test_suite.py
33 files changed, 1,256 insertions(+), 89 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/11314/39
--
To view, visit http://gerrit.cloudera.org:8080/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 39
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-12 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 38:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/657/ : 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/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 38
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 13 Sep 2018 03:07:41 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has uploaded a new patch set (#38). ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..

IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

This patch adds calls to automatically create or remove owner
privileges in the catalog based on the statement.  This is similar to
the existing pattern where after privileges are granted in Sentry,
they are created in the catalog directly instead of pulled from
Sentry.

When object ownership is enabled:
CREATE DATABASE will grant the user OWNER privileges to that database.
ALTER DATABASE SET OWNER will transfer the OWNER privileges to the
new owner.
DROP DATABASE will revoke the OWNER privileges from the owner.
This will apply to DATABASE, TABLE, and VIEW.

Example:
If ownership is enabled, when a table is created, the creator is the
owner, and Sentry will create owner privileges for the created table so
the user can continue working with it without waiting for Sentry
refresh.  Inserts will be available immediately.

Testing:
- Created new custom cluster tests for object ownership

Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
---
M bin/create-test-configuration.sh
M bin/impala-config.sh
M common/thrift/JniCatalog.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/resources/mysql-hive-site.xml.template
M fe/src/test/resources/postgresql-hive-site.xml.template
M fe/src/test/resources/sentry-site.xml.template
A fe/src/test/resources/sentry-site_no_oo.xml.template
A fe/src/test/resources/sentry-site_oo.xml.template
A fe/src/test/resources/sentry-site_oo_nogrant.xml.template
M testdata/bin/run-sentry-service.sh
M tests/authorization/test_grant_revoke.py
A tests/authorization/test_owner_privileges.py
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_test_suite.py
33 files changed, 1,256 insertions(+), 89 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/11314/38
--
To view, visit http://gerrit.cloudera.org:8080/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 38
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 37:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2869
PS33, Line 2869:*/
> Whar if the new type is added later? This code will silently break then.
Done


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2876
PS33, Line 2876:   if (owner != null) {
> The question is why are incrementing catalog version twice here?
Looking around the code, this seems to be the pattern.  I don't think I should 
get the version assigned to removedPrivilege and assign it to owner as well.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 37
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 13 Sep 2018 00:17:46 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-12 Thread Anonymous Coward (Code Review)

Anonymous Coward #424 has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 37:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11314/37/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/37/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2918
PS37, Line 2918: 
owner.setCatalogVersion(catalog_.incrementAndGetCatalogVersion());
I'm somewhat concerned that you modify owner (which is a shared object) without 
holding any locks. Is it safe here?


http://gerrit.cloudera.org:8080/#/c/11314/37/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3165
PS37, Line 3165: Lists.newArrayListWithExpectedSize(privileges.size());
IMO new ArrayList<>(privileges.size()) is cleaner, but if this is an 
established precedent it is ok.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 37
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 12 Sep 2018 22:32:45 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 37:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2873
PS33, Line 2873: try {
> It is but you perform multiple operations and other things can intervene in
a lock is held around both in the latest


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3153
PS33, Line 3153:* Grants or revokes one or more privileges to/from a Sentry 
role on behalf of the
> This was the old pattern because you couldn't use <> in older versions of J
fair points, and we can/should move. however, I'd prefer to keep the file(s) 
consistent and make the move in a separate change.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 37
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 12 Sep 2018 22:03:01 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-12 Thread Anonymous Coward (Code Review)

Anonymous Coward #424 has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 33:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1641
PS33, Line 1641:   public User addUserIfNotExists(String owner, 
Reference existingUser) {
> Because one of the calls needs to know if it's an existing user or not.
What do you think about returning a Pair?


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1644
PS33, Line 1644:   User user = getAuthPolicy().getUser(owner);
> versionLock_ cannot be upgraded to write lock.
Yeah, you are right, it isn't upgradeable, but this is the most common case, so 
you can do a quick check with read lock first and if it isn't there, get a 
write lock and do a full thing. I am not sure how important this lock is.


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2869
PS33, Line 2869:   ownerType == PrincipalType.ROLE ? 
TPrincipalType.ROLE : TPrincipalType.USER);
> Not currently.
Whar if the new type is added later? This code will silently break then.


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2873
PS33, Line 2873:   PrincipalPrivilege removedPrivilege = 
catalog_.getAuthPolicy()
> removePrivilege is synchronized.
It is but you perform multiple operations and other things can intervene in 
between. Is it safe?


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2876
PS33, Line 2876:   
owner.setCatalogVersion(catalog_.incrementAndGetCatalogVersion());
> Because those are distinct objects in the catalog.
The question is why are incrementing catalog version twice here?


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3153
PS33, Line 3153: List removedGrantOptPrivileges = 
Lists.newArrayList();
> Lists.newArrayList seems to be the pattern.  Added size.
This was the old pattern because you couldn't use <> in older versions of Java. 
Now it is rather useless and doesn't allow you to specify initial size.


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3153
PS33, Line 3153: List removedGrantOptPrivileges = 
Lists.newArrayList();
> we've been sticking to newArrayList to keep consistency. we could move to a
The reason Lists.newArrayList() was used in older versions of java was to avoid 
ugly type casts which now you don't have to do. The more serious reason to use 
new ArrayList() directly is because it allows you to specify the initial array 
size that can be better then default.


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3195
PS33, Line 3195:   } else if (privileges.get(0).isHas_grant_opt()) {
> No, the parser should not allow statements without privileges.
Would be nice to add a comment and a precondition



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 33
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 12 Sep 2018 21:56:45 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-12 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 37:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/654/ : 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/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 37
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 12 Sep 2018 20:55:10 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 37: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 37
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 12 Sep 2018 20:37:09 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 37:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1837
PS33, Line 1837:   }
> I was thinking of the case where an impalad has a table of the same name th
Done


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2868
PS33, Line 2868:* and updating the privileges for that owner.
> true, but there are two operations on that cache: 1) get L2868 and 2) mutat
Done


http://gerrit.cloudera.org:8080/#/c/11314/36/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/36/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@220
PS36, Line 220: 6
> nit: update
Done


http://gerrit.cloudera.org:8080/#/c/11314/36/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1025
PS36, Line 1025:* can be used to try adding the owner privilege again.
> state locking assumptions.
Done


http://gerrit.cloudera.org:8080/#/c/11314/36/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2868
PS36, Line 2868:  and updating the privileges for that
> pls explain why the lock is held in the comment.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 37
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 12 Sep 2018 20:20:11 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has uploaded a new patch set (#37). ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..

IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

This patch adds calls to automatically create or remove owner
privileges in the catalog based on the statement.  This is similar to
the existing pattern where after privileges are granted in Sentry,
they are created in the catalog directly instead of pulled from
Sentry.

When object ownership is enabled:
CREATE DATABASE will grant the user OWNER privileges to that database.
ALTER DATABASE SET OWNER will transfer the OWNER privileges to the
new owner.
DROP DATABASE will revoke the OWNER privileges from the owner.
This will apply to DATABASE, TABLE, and VIEW.

Example:
If ownership is enabled, when a table is created, the creator is the
owner, and Sentry will create owner privileges for the created table so
the user can continue working with it without waiting for Sentry
refresh.  Inserts will be available immediately.

Testing:
- Created new custom cluster tests for object ownership

Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
---
M bin/create-test-configuration.sh
M bin/impala-config.sh
M common/thrift/JniCatalog.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/resources/mysql-hive-site.xml.template
M fe/src/test/resources/postgresql-hive-site.xml.template
M fe/src/test/resources/sentry-site.xml.template
A fe/src/test/resources/sentry-site_no_oo.xml.template
A fe/src/test/resources/sentry-site_oo.xml.template
A fe/src/test/resources/sentry-site_oo_nogrant.xml.template
M testdata/bin/run-sentry-service.sh
M tests/authorization/test_grant_revoke.py
A tests/authorization/test_owner_privileges.py
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_test_suite.py
33 files changed, 1,251 insertions(+), 89 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/11314/37
--
To view, visit http://gerrit.cloudera.org:8080/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 37
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 36:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1837
PS33, Line 1837:   updateOwnerPrivileges(newTable.getDbName(), 
newTable.getTableName(),
> But even with multiple impalad that have their own catalog.  If this call d
I was thinking of the case where an impalad has a table of the same name that 
was dropped, but where the drop was not yet reflected in its cache. so it will 
see the table, try to drop it, then try to create it. fwict, the owner of T1's  
operation could then be applied here, which would be odd. unlikely, but odd if 
its possible.


http://gerrit.cloudera.org:8080/#/c/11314/36/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/36/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@220
PS36, Line 220: 5
nit: update


http://gerrit.cloudera.org:8080/#/c/11314/36/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1025
PS36, Line 1025:* can be used to try adding the owner privilege again.
state locking assumptions.


http://gerrit.cloudera.org:8080/#/c/11314/36/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2868
PS36, Line 2868: catalog_.getLock().writeLock().lock();
pls explain why the lock is held in the comment.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 36
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 12 Sep 2018 20:07:23 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-12 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 36:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/652/ : 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/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 36
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 12 Sep 2018 19:44:30 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has uploaded a new patch set (#36). ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..

IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

This patch adds calls to automatically create or remove owner
privileges in the catalog based on the statement.  This is similar to
the existing pattern where after privileges are granted in Sentry,
they are created in the catalog directly instead of pulled from
Sentry.

When object ownership is enabled:
CREATE DATABASE will grant the user OWNER privileges to that database.
ALTER DATABASE SET OWNER will transfer the OWNER privileges to the
new owner.
DROP DATABASE will revoke the OWNER privileges from the owner.
This will apply to DATABASE, TABLE, and VIEW.

Example:
If ownership is enabled, when a table is created, the creator is the
owner, and Sentry will create owner privileges for the created table so
the user can continue working with it without waiting for Sentry
refresh.  Inserts will be available immediately.

Testing:
- Created new custom cluster tests for object ownership

Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
---
M bin/create-test-configuration.sh
M bin/impala-config.sh
M common/thrift/JniCatalog.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/resources/mysql-hive-site.xml.template
M fe/src/test/resources/postgresql-hive-site.xml.template
M fe/src/test/resources/sentry-site.xml.template
A fe/src/test/resources/sentry-site_no_oo.xml.template
A fe/src/test/resources/sentry-site_oo.xml.template
A fe/src/test/resources/sentry-site_oo_nogrant.xml.template
M testdata/bin/run-sentry-service.sh
M tests/authorization/test_grant_revoke.py
A tests/authorization/test_owner_privileges.py
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_test_suite.py
33 files changed, 1,246 insertions(+), 88 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/11314/36
--
To view, visit http://gerrit.cloudera.org:8080/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 36
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 33:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1337
PS33, Line 1337: updateOwnerPrivileges(db.getName(), /* tableName */ null, 
params.server_name,
> It does so via that metastoreDdlLock_ afaict. However, updating privs here
My main concern was the locks that were part of the privilege update and having 
a deadlock because someone gets these in a different order.   Since Sentry has 
it's own locks, I wanted to keep them separate from the HMS locks.


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1837
PS33, Line 1837: updateOwnerPrivileges(newTable.getDbName(), 
newTable.getTableName(),
> there are possibly multiple impalads, each with multiple, uncoordinated req
But even with multiple impalad that have their own catalog.  If this call 
doesn't return, because of the sleep, the other impalad would not have the 
object in it's catalog to be able to drop.  The catalog update would happen 
after the create call is done afaik.


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2896
PS33, Line 2896:   Reference existingUser = new Reference<>();
> this was my suggestion.
Done


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3145
PS33, Line 3145:   private void grantRevokeRolePrivilege(User requestingUser,
> fwict, that lock is acquired when you drill through grantRolePrivs... while
Done


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3153
PS33, Line 3153: List removedGrantOptPrivileges = 
Lists.newArrayList();
> we've been sticking to newArrayList to keep consistency. we could move to a
Done


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3171
PS33, Line 3171:   addedRolePrivileges = Lists.newArrayList();
> here for consistency
Done


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3178
PS33, Line 3178: List updatedPrivs = Lists.newArrayList();
> here for consistency
Done


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3183
PS33, Line 3183: List removedPrivs = Lists.newArrayList();
> here for consistency
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 33
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 12 Sep 2018 17:58:03 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Vuk Ercegovac has posted comments on this change. (
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET
OWNER
..

Patch Set 35:

(9 comments)

thx for the changes. I think the somewhat new issue here is that we're updating
catalogd's replica of two sources (sentry and hms). ordinary path is to update
hms and let sentry push.

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1337
PS33, Line 1337: }
> The catalog should prevent creating the database with the same name at the
It does so via that metastoreDdlLock_ afaict. However, updating privs here is
outside of that lock, which is held on L1312. The pattern in that block is: 1)
do hms op, 2) do local catalog op. Just wondering why these privs are not also
updated under that lock?

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1837
PS33, Line 1837: }
> I don't think this will happen because these calls are in catalogd. create
there are possibly multiple impalads, each with multiple, uncoordinated
requests. also, these calls are synchronously returned to the impalad... there
is no wait for all catalogs to be sync'd unless sync_ddl is used (but we can't
assume that).

if a sleep was put before this line, could we see that sequence: this thread
creates the table and sleeps, another thread drops the table and creates a new
one with the same name, then this thread updates owner privs to the owner of
the original request which may differ from the more recently created table.

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2868
PS33, Line 2868: try {
> Shouldn't be required since it's accessing CatalogObjectCache which is supp
true, but there are two operations on that cache: 1) get L2868 and 2) mutate
L2873. what happens with operations that are interleaved between these two?

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2896
PS33, Line 2896: PrincipalPrivilege cPrivilege;
> This code has changed slightly in the latest patch. The reference for exis
this was my suggestion.

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3145
PS33, Line 3145: /**
> I think the versionLock_ is sufficient.
fwict, that lock is acquired when you drill through grantRolePrivs... while
that method seems atomic, its not. there is a loop there and each iter in the
loop gets that lock. that seems to be an existing issue, unrelated to this
change.
were it atomic, then the rest of this method is not mutating these objects but
just filling in the response.

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3153
PS33, Line 3153: verifySentryServiceEnabled();
> Lists.newArrayList seems to be the pattern. Added size.
we've been sticking to newArrayList to keep consistency. we could move to a
different style, but lets do that in a separate pass and lets not introduce
inconsistent styles in the same file.

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3171
PS33, Line 3171: // addedRolePrivileges parameter will contain a list of
new privileges without the
> Lists.newArrayList seems to be the pattern. Added size.
here for consistency

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3178
PS33, Line 3178: .revokeRolePrivileges(requestingUser, roleName,
privileges,
> Lists.newArrayList seems to be the pattern. Added size.
here for consistency

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3183
PS33, Line 3183: List updatedPrivs =
> Lists.newArrayList seems to be the pattern. Added size.
here for consistency

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 35
Gerrit-Owner: Adam Holley
Gerrit-Reviewer: Adam Holley
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Wed, 12 Sep 2018 17:42:39 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-12 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 35: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 35
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 12 Sep 2018 06:45:33 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-11 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 35:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/648/ : 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/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 35
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 12 Sep 2018 03:28:34 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-11 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 35:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3147/ 
DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 35
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Wed, 12 Sep 2018 02:53:33 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 33:

(36 comments)

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java:

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java@56
PS30, Line 56: serverName_ = analyzer.getServerName();
> Here and in all other similar places please consider interning the string (
Done


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java:

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java@56
PS33, Line 56: serverName_ = analyzer.getServerName();
> Here and in all other similar places you should consider using string inter
Done


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1639
PS33, Line 1639:* can be added for a user. example: owner privileges.
> Please document locking used.
versionLock_ is documented at the top of the file.  Is there something 
additional you are looking for?


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1641
PS33, Line 1641:   public User addUserIfNotExists(String owner, 
Reference existingUser) {
> Why do you need Reference here?
Because one of the calls needs to know if it's an existing user or not.


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1644
PS33, Line 1644:   User user = getAuthPolicy().getUser(owner);
> I guess in most cases you get an existing user - would it make sense to get
versionLock_ cannot be upgraded to write lock.


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1009
PS33, Line 1009:   /**
> Style: The first sentence of Javadoc is used in special ways - it should be
Done


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1025
PS33, Line 1025:   private void updateOwnerPrivileges(String databaseName, 
String tableName,
> Style: you may consider having multiple user-facing functions which are sim
This was originally multiple functions and a previous review suggested to 
consolidate.


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1037
PS33, Line 1037: if(oldOwner != null && oldOwner.length() > 0) {
> Style: it is better to use isEmpty() rather then compare size with 0.
Done


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1038
PS33, Line 1038:   removePrivilegeFromCatalog(oldOwner, oldOwnerType, 
filter, resp);
> Can this fail?
Yes.  However, there isn't currently an easy way to notify the user of any 
error for these privilege updates.  Throwing an exception would probably cause 
the other metadata catalog updates to fail even though the HMS updates were 
successful.


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1337
PS33, Line 1337: updateOwnerPrivileges(db.getName(), /* tableName */ null, 
params.server_name,
> Can someone else create the database with the same name at the same time so
The catalog should prevent creating the database with the same name at the same 
time.  Any exceptions thrown occur before the privilege update calls.


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1837
PS33, Line 1837: updateOwnerPrivileges(newTable.getDbName(), 
newTable.getTableName(),
> Is it possible that another threadd drops the table and creates a new one w
I don't think this will happen because these calls are in catalogd.  creates 
and drops are initiated by impalad.  The create on the new thread won't be 
allowed to happen until the drop table completes in the catalog, returns to 
impalad, and all catalogs are synced.


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1849
PS33, Line 1849:

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has uploaded a new patch set (#35). ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..

IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

This patch adds calls to automatically create or remove owner
privileges in the catalog based on the statement.  This is similar to
the existing pattern where after privileges are granted in Sentry,
they are created in the catalog directly instead of pulled from
Sentry.

When object ownership is enabled:
CREATE DATABASE will grant the user OWNER privileges to that database.
ALTER DATABASE SET OWNER will transfer the OWNER privileges to the
new owner.
DROP DATABASE will revoke the OWNER privileges from the owner.
This will apply to DATABASE, TABLE, and VIEW.

Example:
If ownership is enabled, when a table is created, the creator is the
owner, and Sentry will create owner privileges for the created table so
the user can continue working with it without waiting for Sentry
refresh.  Inserts will be available immediately.

Testing:
- Created new custom cluster tests for object ownership

Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
---
M bin/create-test-configuration.sh
M bin/impala-config.sh
M common/thrift/JniCatalog.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/resources/mysql-hive-site.xml.template
M fe/src/test/resources/postgresql-hive-site.xml.template
M fe/src/test/resources/sentry-site.xml.template
A fe/src/test/resources/sentry-site_no_oo.xml.template
A fe/src/test/resources/sentry-site_oo.xml.template
A fe/src/test/resources/sentry-site_oo_nogrant.xml.template
M testdata/bin/run-sentry-service.sh
M tests/authorization/test_grant_revoke.py
A tests/authorization/test_owner_privileges.py
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_test_suite.py
33 files changed, 1,244 insertions(+), 89 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/11314/35
--
To view, visit http://gerrit.cloudera.org:8080/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 35
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-11 Thread Anonymous Coward (Code Review)

Anonymous Coward #424 has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 33:

(37 comments)

http://gerrit.cloudera.org:8080/#/c/11314/30/common/thrift/JniCatalog.thrift
File common/thrift/JniCatalog.thrift:

http://gerrit.cloudera.org:8080/#/c/11314/30/common/thrift/JniCatalog.thrift@76
PS30, Line 76:
> Agreed but a bigger change than part of this cr.  Opened https://issues.apa
Done


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java:

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java@56
PS30, Line 56: serverName_ = analyzer.getServerName();
Here and in all other similar places please consider interning the string (it 
should be the same string everywhere).


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java:

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java@56
PS33, Line 56: serverName_ = analyzer.getServerName();
Here and in all other similar places you should consider using string interning 
since serverName is the same everywhere.


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1639
PS33, Line 1639:* can be added for a user. example: owner privileges.
Please document locking used.


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1641
PS33, Line 1641:   public User addUserIfNotExists(String owner, 
Reference existingUser) {
Why do you need Reference here?


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1644
PS33, Line 1644:   User user = getAuthPolicy().getUser(owner);
I guess in most cases you get an existing user - would it make sense to get 
readlock and upgrade it to write lock if this is a new user?


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1009
PS33, Line 1009:   /**
Style: The first sentence of Javadoc is used in special ways - it should be a 
very short description of what the method is doing.


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1025
PS33, Line 1025:   private void updateOwnerPrivileges(String databaseName, 
String tableName,
Style: you may consider having multiple user-facing functions which are simpler 
- e.g. addTableOwnerPrivilege, etc and they can all call your fancy common 
function. This will help with many call sites where you have to specify which 
null means what.


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1037
PS33, Line 1037: if(oldOwner != null && oldOwner.length() > 0) {
Style: it is better to use isEmpty() rather then compare size with 0.


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1038
PS33, Line 1038:   removePrivilegeFromCatalog(oldOwner, oldOwnerType, 
filter, resp);
Can this fail?


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1337
PS33, Line 1337: updateOwnerPrivileges(db.getName(), /* tableName */ null, 
params.server_name,
Can someone else create the database with the same name at the same time so 
that you'll drop owner privileges for the wrong database?


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1837
PS33, Line 1837: updateOwnerPrivileges(newTable.getDbName(), 
newTable.getTableName(),
Is it possible that another threadd drops the table and creates a new one with 
the same name and you'll update owner privileges on the wrong table?


http://gerrit.cloudera.org:8080/#/c/11314/33/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1849
PS33, Line 1849: TPrivilege privilege = 
createDatabaseOwnerPrivilegeFilter(databaseName, serverName);
Can we do all of this in constructor?

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-11 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 34:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/642/ : 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/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 34
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 11 Sep 2018 19:21:42 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-11 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 33:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/641/ : 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/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 33
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 11 Sep 2018 18:57:32 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has uploaded a new patch set (#34). ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..

IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

This patch adds calls to automatically create or remove owner
privileges in the catalog based on the statement.  This is similar to
the existing pattern where after privileges are granted in Sentry,
they are created in the catalog directly instead of pulled from
Sentry.

When object ownership is enabled:
CREATE DATABASE will grant the user OWNER privileges to that database.
ALTER DATABASE SET OWNER will transfer the OWNER privileges to the
new owner.
DROP DATABASE will revoke the OWNER privileges from the owner.
This will apply to DATABASE, TABLE, and VIEW.

Example:
If ownership is enabled, when a table is created, the creator is the
owner, and Sentry will create owner privileges for the created table so
the user can continue working with it without waiting for Sentry
refresh.  Inserts will be available immediately.

Testing:
- Created new custom cluster tests for object ownership

Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
---
M bin/create-test-configuration.sh
M bin/impala-config.sh
M common/thrift/JniCatalog.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/resources/mysql-hive-site.xml.template
M fe/src/test/resources/postgresql-hive-site.xml.template
M fe/src/test/resources/sentry-site.xml.template
A fe/src/test/resources/sentry-site_no_oo.xml.template
A fe/src/test/resources/sentry-site_oo.xml.template
A fe/src/test/resources/sentry-site_oo_nogrant.xml.template
M testdata/bin/run-sentry-service.sh
M tests/authorization/test_grant_revoke.py
A tests/authorization/test_owner_privileges.py
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_test_suite.py
33 files changed, 1,233 insertions(+), 87 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/11314/34
--
To view, visit http://gerrit.cloudera.org:8080/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 34
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has uploaded a new patch set (#33). ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..

IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

This patch adds calls to automatically create or remove owner
privileges in the catalog based on the statement.  This is similar to
the existing pattern where after privileges are granted in Sentry,
they are created in the catalog directly instead of pulled from
Sentry.

When object ownership is enabled:
CREATE DATABASE will grant the user OWNER privileges to that database.
ALTER DATABASE SET OWNER will transfer the OWNER privileges to the
new owner.
DROP DATABASE will revoke the OWNER privileges from the owner.
This will apply to DATABASE, TABLE, and VIEW.

Example:
If ownership is enabled, when a table is created, the creator is the
owner, and Sentry will create owner privileges for the created table so
the user can continue working with it without waiting for Sentry
refresh.  Inserts will be available immediately.

Testing:
- Created new custom cluster tests for object ownership

Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
---
M bin/create-test-configuration.sh
M bin/impala-config.sh
M common/thrift/JniCatalog.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/resources/mysql-hive-site.xml.template
M fe/src/test/resources/postgresql-hive-site.xml.template
M fe/src/test/resources/sentry-site.xml.template
A fe/src/test/resources/sentry-site_no_oo.xml.template
A fe/src/test/resources/sentry-site_oo.xml.template
A fe/src/test/resources/sentry-site_oo_nogrant.xml.template
M testdata/bin/run-sentry-service.sh
M tests/authorization/test_grant_revoke.py
A tests/authorization/test_owner_privileges.py
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_test_suite.py
33 files changed, 1,232 insertions(+), 87 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/11314/33
--
To view, visit http://gerrit.cloudera.org:8080/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 33
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 32:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/11314/30/common/thrift/JniCatalog.thrift
File common/thrift/JniCatalog.thrift:

http://gerrit.cloudera.org:8080/#/c/11314/30/common/thrift/JniCatalog.thrift@76
PS30, Line 76:
> add todo's to remove and reference the jira.
Done


http://gerrit.cloudera.org:8080/#/c/11314/32/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/11314/32/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1639
PS32, Line 1639: .  For example
> nit: condense to (example: owner pr..).
Done


http://gerrit.cloudera.org:8080/#/c/11314/32/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1641
PS32, Line 1641: org.apache.impala.catalog.User
> no need for fully qualified name here (and below on L1644)
Done


http://gerrit.cloudera.org:8080/#/c/11314/32/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/32/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2883
PS32, Line 2883: modifying
> removing
Done


http://gerrit.cloudera.org:8080/#/c/11314/32/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2911
PS32, Line 2911: modifying
> adding
Done


http://gerrit.cloudera.org:8080/#/c/11314/32/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:

http://gerrit.cloudera.org:8080/#/c/11314/32/fe/src/main/java/org/apache/impala/util/SentryProxy.java@222
PS32, Line 222: if (resetVersions_) {
> something looks off with this change: prior, this branch was run only when
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 32
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 11 Sep 2018 18:28:46 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-11 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 32: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 32
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 11 Sep 2018 18:26:22 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-11 Thread Vuk Ercegovac (Code Review)

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 32:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/11314/30/common/thrift/JniCatalog.thrift
File common/thrift/JniCatalog.thrift:

http://gerrit.cloudera.org:8080/#/c/11314/30/common/thrift/JniCatalog.thrift@76
PS30, Line 76:
> Agreed but a bigger change than part of this cr.  Opened https://issues.apa
add todo's to remove and reference the jira.


http://gerrit.cloudera.org:8080/#/c/11314/32/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/11314/32/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1639
PS32, Line 1639: .  For example
nit: condense to (example: owner pr..).


http://gerrit.cloudera.org:8080/#/c/11314/32/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1641
PS32, Line 1641: org.apache.impala.catalog.User
no need for fully qualified name here (and below on L1644)


http://gerrit.cloudera.org:8080/#/c/11314/32/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/32/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2883
PS32, Line 2883: modifying
removing


http://gerrit.cloudera.org:8080/#/c/11314/32/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2911
PS32, Line 2911: modifying
adding


http://gerrit.cloudera.org:8080/#/c/11314/32/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:

http://gerrit.cloudera.org:8080/#/c/11314/32/fe/src/main/java/org/apache/impala/util/SentryProxy.java@222
PS32, Line 222: if (resetVersions_) {
something looks off with this change: prior, this branch was run only when the 
user already existed. that can be fixed by adding a Reference bool parameter to 
the method indicating if the user was added or not.

the other thing I'll note is that prior, there was less contention for the 
write lock. since we have multiple ways to add a user, this will compete with 
all other operations. this may be something to look at again when we stress it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 32
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 11 Sep 2018 17:50:30 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-11 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 32:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/638/ : 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/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 32
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 11 Sep 2018 15:11:53 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-11 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 32:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3143/ 
DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 32
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 11 Sep 2018 14:38:46 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-11 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 31: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3142/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 31
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 11 Sep 2018 09:20:47 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-10 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 31:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/636/ : 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/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 31
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 11 Sep 2018 05:54:27 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-10 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 31:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3142/ 
DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 31
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 11 Sep 2018 05:20:18 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has uploaded a new patch set (#31). ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..

IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

This patch adds calls to automatically create or remove owner
privileges in the catalog based on the statement.  This is similar to
the existing pattern where after privileges are granted in Sentry,
they are created in the catalog directly instead of pulled from
Sentry.

When object ownership is enabled:
CREATE DATABASE will grant the user OWNER privileges to that database.
ALTER DATABASE SET OWNER will transfer the OWNER privileges to the
new owner.
DROP DATABASE will revoke the OWNER privileges from the owner.
This will apply to DATABASE, TABLE, and VIEW.

Example:
If ownership is enabled, when a table is created, the creator is the
owner, and Sentry will create owner privileges for the created table so
the user can continue working with it without waiting for Sentry
refresh.  Inserts will be available immediately.

Testing:
- Created new custom cluster tests for object ownership

Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
---
M bin/create-test-configuration.sh
M bin/impala-config.sh
M common/thrift/JniCatalog.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/resources/mysql-hive-site.xml.template
M fe/src/test/resources/postgresql-hive-site.xml.template
M fe/src/test/resources/sentry-site.xml.template
A fe/src/test/resources/sentry-site_no_oo.xml.template
A fe/src/test/resources/sentry-site_oo.xml.template
A fe/src/test/resources/sentry-site_oo_nogrant.xml.template
M testdata/bin/run-sentry-service.sh
M tests/authorization/test_grant_revoke.py
A tests/authorization/test_owner_privileges.py
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_test_suite.py
33 files changed, 1,219 insertions(+), 87 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/11314/31
--
To view, visit http://gerrit.cloudera.org:8080/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 31
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 30:

(2 comments)

Missed a couple of comments.

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1338
PS30, Line 1338: updateDatabasePrivileges(db.getName(), /* tableName */ 
null, params.server_name,
> Why do we call updateDatabasePrivilege here then?
because we update on dropDatabase.


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1849
PS30, Line 1849:   private synchronized org.apache.impala.catalog.User 
addUserIfNotExists(
> This is really suspicious. There are other writers that call catalog.addUse
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 30
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 11 Sep 2018 04:43:09 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 30:

(7 comments)

Running test builds, will upload latest patch after.

http://gerrit.cloudera.org:8080/#/c/11314/30/common/thrift/JniCatalog.thrift
File common/thrift/JniCatalog.thrift:

http://gerrit.cloudera.org:8080/#/c/11314/30/common/thrift/JniCatalog.thrift@76
PS30, Line 76:
> The server name should be exactly the same everywhere. Having it in multipl
Agreed but a bigger change than part of this cr.  Opened 
https://issues.apache.org/jira/browse/IMPALA-7553


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java:

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java@35
PS30, Line 35:   private String serverName_;
> We don't support anything that doesn't have the same serverName for everyth
Opened https://issues.apache.org/jira/browse/IMPALA-7553


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
File 
fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java:

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java@34
PS30, Line 34:   // Server name needed for privileges. Set during analysis.
> There is no way serverName can change in alterTable event - do we really ne
We need it here to be able to create privileges in catalogd.  Currently 
catalogd does not configure server_name.


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2664
PS30, Line 2664: if (getAuthzConfig().isEnabled()) return 
getAuthzConfig().getServerName();
> Style: if statements should always have {} braces and conform to normal Jav
Done


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java:

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java@39
PS30, Line 39:   // Server name needed for privileges. Set during analysis.
> We only support one global serverName.
https://issues.apache.org/jira/browse/IMPALA-7553


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
File fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java:

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java@193
PS30, Line 193: params.setComment(comment_);
> Is this an unrelated change?
Since the null check was not needed for server_name, I removed this one.


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1033
PS30, Line 1033: if (tableName == null) {
> This is weird - the method is called updateDatabasePrivileges, but here you
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 30
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 11 Sep 2018 02:41:19 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-10 Thread Anonymous Coward (Code Review)

Anonymous Coward #424 has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 30:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/11314/30/common/thrift/JniCatalog.thrift
File common/thrift/JniCatalog.thrift:

http://gerrit.cloudera.org:8080/#/c/11314/30/common/thrift/JniCatalog.thrift@76
PS30, Line 76:
The server name should be exactly the same everywhere. Having it in multiple 
places just asks for trouble.


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java:

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java@35
PS30, Line 35:   private String serverName_;
We don't support anything that doesn't have the same serverName for everything 
and this serverName is statically configured. Having serverName per entity is 
just a waste of memory. At a minimum it should be interned. In practice, it is 
always the string "server1".


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
File 
fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java:

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java@34
PS30, Line 34:   // Server name needed for privileges. Set during analysis.
There is no way serverName can change in alterTable event - do we really need 
it here?


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2664
PS30, Line 2664: if (getAuthzConfig().isEnabled()) return 
getAuthzConfig().getServerName();
Style: if statements should always have {} braces and conform to normal Java 
formatting.

That said you don't need an if statement here at all ands can use ? : construct.


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java:

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java@39
PS30, Line 39:   // Server name needed for privileges. Set during analysis.
We only support one global serverName.


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
File fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java:

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java@193
PS30, Line 193: params.setComment(comment_);
Is this an unrelated change?


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1023
PS30, Line 1023:* the privilege, it will be removed on the next refresh. 
ALTER DATABASE SET OWNER
> My understanding is that the HMS calls will not return until the Sentry cal
HMS calls have a 2-minute timeout after which they return and continue happily


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1033
PS30, Line 1033: if (tableName == null) {
This is weird - the method is called updateDatabasePrivileges, but here you 
handle both database and table privileges? WOuld it be better to be more 
explicit on whether you are dealing with a database or a table?


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1338
PS30, Line 1338: updateDatabasePrivileges(db.getName(), /* tableName */ 
null, params.server_name,
> No because functions do not have privileges.
Why do we call updateDatabasePrivilege here then?


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1849
PS30, Line 1849:   private synchronized org.apache.impala.catalog.User 
addUserIfNotExists(
This is really suspicious. There are other writers that call catalog.addUser() 
which do not go through this code path. I think catalog itself should have 
addUserIfNotExists() method that is coordinated with all addUsers() calls.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-10 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 30: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3139/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 30
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 11 Sep 2018 00:40:17 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 30:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1013
PS30, Line 1013:* the absence, will function correctly without waiting for 
the next refresh.
> How do you deal with the case when the next refresh modifies the privileges
Privileges that are created, should only be created if enabled in Sentry.  So a 
refresh should not change the state except on a failure in Sentry.


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1023
PS30, Line 1023:* the privilege, it will be removed on the next refresh. 
ALTER DATABASE SET OWNER
> I don't think that it will help much in case Sentry is delayed.
My understanding is that the HMS calls will not return until the Sentry calls 
are made.


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1338
PS30, Line 1338: updateDatabasePrivileges(db.getName(), /* tableName */ 
null, params.server_name,
> Do we need to do this for CreateFunction?
No because functions do not have privileges.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 30
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 10 Sep 2018 21:23:10 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-10 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 30:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/632/ : 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/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 30
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 10 Sep 2018 21:10:57 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-10 Thread Anonymous Coward (Code Review)

Anonymous Coward #424 has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 30:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1013
PS30, Line 1013:* the absence, will function correctly without waiting for 
the next refresh.
How do you deal with the case when the next refresh modifies the privileges 
back to the state they were before?


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1023
PS30, Line 1023:* the privilege, it will be removed on the next refresh. 
ALTER DATABASE SET OWNER
I don't think that it will help much in case Sentry is delayed.


http://gerrit.cloudera.org:8080/#/c/11314/30/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1338
PS30, Line 1338: updateDatabasePrivileges(db.getName(), /* tableName */ 
null, params.server_name,
Do we need to do this for CreateFunction?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 30
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Anonymous Coward #424
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 10 Sep 2018 21:07:10 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-10 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 30:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3139/ 
DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 30
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 10 Sep 2018 20:47:44 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has uploaded a new patch set (#30). ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..

IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

This patch adds calls to automatically create or remove owner
privileges in the catalog based on the statement.  This is similar to
the existing pattern where after privileges are granted in Sentry,
they are created in the catalog directly instead of pulled from
Sentry.

When object ownership is enabled:
CREATE DATABASE will grant the user OWNER privileges to that database.
ALTER DATABASE SET OWNER will transfer the OWNER privileges to the
new owner.
DROP DATABASE will revoke the OWNER privileges from the owner.
This will apply to DATABASE, TABLE, and VIEW.

Example:
If ownership is enabled, when a table is created, the creator is the
owner, and Sentry will create owner privileges for the created table so
the user can continue working with it without waiting for Sentry
refresh.  Inserts will be available immediately.

Testing:
- Created new custom cluster tests for object ownership

Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
---
M bin/create-test-configuration.sh
M bin/impala-config.sh
M common/thrift/JniCatalog.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/resources/mysql-hive-site.xml.template
M fe/src/test/resources/postgresql-hive-site.xml.template
M fe/src/test/resources/sentry-site.xml.template
A fe/src/test/resources/sentry-site_no_oo.xml.template
A fe/src/test/resources/sentry-site_oo.xml.template
A fe/src/test/resources/sentry-site_oo_nogrant.xml.template
M testdata/bin/run-sentry-service.sh
M tests/authorization/test_grant_revoke.py
A tests/authorization/test_owner_privileges.py
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_test_suite.py
33 files changed, 1,216 insertions(+), 77 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/11314/30
--
To view, visit http://gerrit.cloudera.org:8080/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 30
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 29:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3165
PS29, Line 3165: rolePrivileges
> are these meant to be 'added'?
Done


http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3207
PS29, Line 3207: privileges.get(0)
> what's special about the first one? is there some homogeneity assumption he
They are homogeneous.  The only time there are ever multiple privileges is in 
the case of column level select privileges.


http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3211
PS29, Line 3211: updatedPrivs.size() - 1
> what's special about the version of the last one? perhaps a helper method w
Done


http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:

http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/util/SentryProxy.java@382
PS29, Line 382:* modified. Returns the removed privileges. Throws an 
exception if there was any error
> update the comment to describe the newly added return parameter.
Done


http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/util/SentryProxy.java@400
PS29, Line 400: new ones added
> make this more specific: "these same privileges are added back to the catal
Done


http://gerrit.cloudera.org:8080/#/c/11314/29/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
File testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test:

http://gerrit.cloudera.org:8080/#/c/11314/29/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test@798
PS29, Line 798: 'functional',''
> perhaps a regex can be applied here to succeed when anything but 'grant_rev
Disabled object ownership by default the dev environment.  Caused other tests 
to fail.  This will be reverted.


http://gerrit.cloudera.org:8080/#/c/11314/29/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test@859
PS29, Line 859: 'functional',''
> same suggestion here to rely on a regex filter
Disabled object ownership by default the dev environment.  Caused other tests 
to fail.  This will be reverted.


http://gerrit.cloudera.org:8080/#/c/11314/26/tests/authorization/test_owner_privileges.py
File tests/authorization/test_owner_privileges.py:

http://gerrit.cloudera.org:8080/#/c/11314/26/tests/authorization/test_owner_privileges.py@132
PS26, Line 132:   @CustomClusterTestSuite.with_ar
> unused, pls remove.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 29
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 10 Sep 2018 20:40:58 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-10 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 29: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3136/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 29
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 10 Sep 2018 18:45:35 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-10 Thread Vuk Ercegovac (Code Review)

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 29:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3165
PS29, Line 3165: rolePrivileges
are these meant to be 'added'?


http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3207
PS29, Line 3207: privileges.get(0)
what's special about the first one? is there some homogeneity assumption here 
that should rather be a param?


http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3211
PS29, Line 3211: updatedPrivs.size() - 1
what's special about the version of the last one? perhaps a helper method will 
explain better what's going on here.


http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/util/SentryProxy.java
File fe/src/main/java/org/apache/impala/util/SentryProxy.java:

http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/util/SentryProxy.java@382
PS29, Line 382:* modified. Returns the removed privileges. Throws an 
exception if there was any error
update the comment to describe the newly added return parameter.


http://gerrit.cloudera.org:8080/#/c/11314/29/fe/src/main/java/org/apache/impala/util/SentryProxy.java@400
PS29, Line 400: new ones added
make this more specific: "these same privileges are added back to the catalog 
with grantoption set to false".
basically, you're modeling an update as a delete/insert.
if I've overstated this, then pls clarify "new".


http://gerrit.cloudera.org:8080/#/c/11314/29/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
File testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test:

http://gerrit.cloudera.org:8080/#/c/11314/29/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test@798
PS29, Line 798: 'functional',''
perhaps a regex can be applied here to succeed when anything but 'grant_rev_db' 
matches? worried that this list will be cumbersome to maintain. looks like 
'show databases' includes a 'like'-based filter.


http://gerrit.cloudera.org:8080/#/c/11314/29/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test@859
PS29, Line 859: 'functional',''
same suggestion here to rely on a regex filter



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 29
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 10 Sep 2018 17:19:59 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-10 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 29:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3136/ 
DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 29
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 10 Sep 2018 14:37:22 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-10 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 29:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/627/ : 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/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 29
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 10 Sep 2018 14:23:40 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has uploaded a new patch set (#29). ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..

IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

This patch adds calls to automatically create or remove owner
privileges in the catalog based on the statement.  This is similar to
the existing pattern where after privileges are granted in Sentry,
they are created in the catalog directly instead of pulled from
Sentry.

The sentry-site.xml and hive-site.xml template files have been updated
to enable usage of object ownership by default for easier development
with a secured impala.

When object ownership is enabled:
CREATE DATABASE will grant the user OWNER privileges to that database.
ALTER DATABASE SET OWNER will transfer the OWNER privileges to the
new owner.
DROP DATABASE will revoke the OWNER privileges from the owner.
This will apply to DATABASE, TABLE, and VIEW.

Example:
If ownership is enabled, when a table is created, the creator is the
owner, and Sentry will create owner privileges for the created table so
the user can continue working with it without waiting for Sentry
refresh.  Inserts will be available immediately.

Testing:
- Created new custom cluster tests for object ownership

Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
---
M bin/create-test-configuration.sh
M bin/impala-config.sh
M common/thrift/JniCatalog.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/resources/mysql-hive-site.xml.template
M fe/src/test/resources/postgresql-hive-site.xml.template
M fe/src/test/resources/sentry-site.xml.template
A fe/src/test/resources/sentry-site_no_oo.xml.template
A fe/src/test/resources/sentry-site_oo_nogrant.xml.template
M testdata/bin/run-sentry-service.sh
M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_grant_revoke.py
A tests/authorization/test_owner_privileges.py
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_test_suite.py
33 files changed, 1,192 insertions(+), 66 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/11314/29
--
To view, visit http://gerrit.cloudera.org:8080/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 29
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-10 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 28:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/11314/28/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/28/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1492
PS28, Line 1492:   updateDatabasePrivileges(table.getDb().getName(), 
table.getName(), params.server_name,
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/11314/28/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3183
PS28, Line 3183:   removedGrantOptPrivileges = 
catalog_.getSentryProxy().revokeRolePrivileges(requestingUser,
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/11314/28/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:

http://gerrit.cloudera.org:8080/#/c/11314/28/tests/authorization/test_grant_revoke.py@32
PS28, Line 32: '
flake8: E501 line too long (91 > 90 characters)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 28
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 10 Sep 2018 10:17:43 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-10 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 28:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/626/ : 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/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 28
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 10 Sep 2018 06:25:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-09 Thread Adam Holley (Code Review)

Adam Holley has uploaded a new patch set (#28). ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..

IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

This patch adds calls to automatically create or remove owner
privileges in the catalog based on the statement.  This is similar to
the existing pattern where after privileges are granted in Sentry,
they are created in the catalog directly instead of pulled from
Sentry.

The sentry-site.xml and hive-site.xml template files have been updated
to enable usage of object ownership by default for easier development
with a secured impala.

When object ownership is enabled:
CREATE DATABASE will grant the user OWNER privileges to that database.
ALTER DATABASE SET OWNER will transfer the OWNER privileges to the
new owner.
DROP DATABASE will revoke the OWNER privileges from the owner.
This will apply to DATABASE, TABLE, and VIEW.

Example:
If ownership is enabled, when a table is created, the creator is the
owner, and Sentry will create owner privileges for the created table so
the user can continue working with it without waiting for Sentry
refresh.  Inserts will be available immediately.

Testing:
- Created new custom cluster tests for object ownership

Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
---
M bin/create-test-configuration.sh
M bin/impala-config.sh
M common/thrift/JniCatalog.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/resources/mysql-hive-site.xml.template
M fe/src/test/resources/postgresql-hive-site.xml.template
M fe/src/test/resources/sentry-site.xml.template
A fe/src/test/resources/sentry-site_no_oo.xml.template
A fe/src/test/resources/sentry-site_oo_nogrant.xml.template
M testdata/bin/run-sentry-service.sh
M testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_grant_revoke.py
A tests/authorization/test_owner_privileges.py
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_test_suite.py
33 files changed, 1,189 insertions(+), 66 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/11314/28
--
To view, visit http://gerrit.cloudera.org:8080/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 28
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-07 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 27: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3132/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 27
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 07 Sep 2018 22:08:43 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-07 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 27:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3132/ 
DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 27
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 07 Sep 2018 18:01:51 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-07 Thread Vuk Ercegovac (Code Review)

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 27: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 27
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 07 Sep 2018 18:00:58 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-07 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 27:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/616/ : 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/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 27
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 07 Sep 2018 17:03:47 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has uploaded a new patch set (#27). ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..

IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

This patch adds calls to automatically create or remove owner
privileges in the catalog based on the statement.  This is similar to
the existing pattern where after privileges are granted in Sentry,
they are created in the catalog directly instead of pulled from
Sentry.

The sentry-site.xml and hive-site.xml template files have been updated
to enable usage of object ownership by default for easier development
with a secured impala.

When object ownership is enabled:
CREATE DATABASE will grant the user OWNER privileges to that database.
ALTER DATABASE SET OWNER will transfer the OWNER privileges to the
new owner.
DROP DATABASE will revoke the OWNER privileges from the owner.
This will apply to DATABASE, TABLE, and VIEW.

Example:
If ownership is enabled, when a table is created, the creator is the
owner, and Sentry will create owner privileges for the created table so
the user can continue working with it without waiting for Sentry
refresh.  Inserts will be available immediately.

Testing:
- Created new custom cluster tests for object ownership

Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
---
M bin/create-test-configuration.sh
M bin/impala-config.sh
M common/thrift/JniCatalog.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/resources/mysql-hive-site.xml.template
M fe/src/test/resources/postgresql-hive-site.xml.template
M fe/src/test/resources/sentry-site.xml.template
A fe/src/test/resources/sentry-site_no_oo.xml.template
A fe/src/test/resources/sentry-site_oo_nogrant.xml.template
M testdata/bin/run-sentry-service.sh
A tests/authorization/test_owner_privileges.py
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_test_suite.py
31 files changed, 1,041 insertions(+), 41 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/11314/27
--
To view, visit http://gerrit.cloudera.org:8080/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 27
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-07 Thread Vuk Ercegovac (Code Review)

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 26: Code-Review+2

(2 comments)

couple more nits/clarifications.

http://gerrit.cloudera.org:8080/#/c/11314/26/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/26/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1019
PS26, Line 1019:* Sentry refresh.
Correct, Impala's view will diverge from Sentry's. However, per my 
understanding, the situation is worse since Impala will adopt Sentry's view, 
which may or may not be the same as the HMS view (and may never be in sync due 
to errors). When this situation comes up, the workaround is to try the 
operation again as suggested here on L1022.


http://gerrit.cloudera.org:8080/#/c/11314/26/tests/authorization/test_owner_privileges.py
File tests/authorization/test_owner_privileges.py:

http://gerrit.cloudera.org:8080/#/c/11314/26/tests/authorization/test_owner_privileges.py@132
PS26, Line 132:   def restart_first_impalad(cls):
unused, pls remove.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 26
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 07 Sep 2018 16:23:45 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-07 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 26:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/615/ : 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/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 26
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 07 Sep 2018 15:33:12 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has uploaded a new patch set (#26). ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..

IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

This patch adds calls to automatically create or remove owner
privileges in the catalog based on the statement.  This is similar to
the existing pattern where after privileges are granted in Sentry,
they are created in the catalog directly instead of pulled from
Sentry.

The sentry-site.xml and hive-site.xml template files have been updated
to enable usage of object ownership by default for easier development
with a secured impala.

When object ownership is enabled:
CREATE DATABASE will grant the user OWNER privileges to that database.
ALTER DATABASE SET OWNER will transfer the OWNER privileges to the
new owner.
DROP DATABASE will revoke the OWNER privileges from the owner.
This will apply to DATABASE, TABLE, and VIEW.

Example:
If ownership is enabled, when a table is created, the creator is the
owner, and Sentry will create owner privileges for the created table so
the user can continue working with it without waiting for Sentry
refresh.  Inserts will be available immediately.

Testing:
- Created new custom cluster tests for object ownership

Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
---
M bin/create-test-configuration.sh
M bin/impala-config.sh
M common/thrift/JniCatalog.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/resources/mysql-hive-site.xml.template
M fe/src/test/resources/postgresql-hive-site.xml.template
M fe/src/test/resources/sentry-site.xml.template
A fe/src/test/resources/sentry-site_no_oo.xml.template
A fe/src/test/resources/sentry-site_oo_nogrant.xml.template
M testdata/bin/run-sentry-service.sh
A tests/authorization/test_owner_privileges.py
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_test_suite.py
31 files changed, 1,047 insertions(+), 41 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/11314/26
--
To view, visit http://gerrit.cloudera.org:8080/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 26
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-07 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 25:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11314/25/tests/authorization/test_owner_privileges.py
File tests/authorization/test_owner_privileges.py:

http://gerrit.cloudera.org:8080/#/c/11314/25/tests/authorization/test_owner_privileges.py@305
PS25, Line 305: +
flake8: E225 missing whitespace around operator



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 25
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 07 Sep 2018 14:58:01 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-07 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 25:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/614/ : 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/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 25
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 07 Sep 2018 14:53:01 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has uploaded a new patch set (#25). ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..

IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

This patch adds calls to automatically create or remove owner
privileges in the catalog based on the statement.  This is similar to
the existing pattern where after privileges are granted in Sentry,
they are created in the catalog directly instead of pulled from
Sentry.

The sentry-site.xml and hive-site.xml template files have been updated
to enable usage of object ownership by default for easier development
with a secured impala.

When object ownership is enabled:
CREATE DATABASE will grant the user OWNER privileges to that database.
ALTER DATABASE SET OWNER will transfer the OWNER privileges to the
new owner.
DROP DATABASE will revoke the OWNER privileges from the owner.
This will apply to DATABASE, TABLE, and VIEW.

Example:
If ownership is enabled, when a table is created, the creator is the
owner, and Sentry will create owner privileges for the created table so
the user can continue working with it without waiting for Sentry
refresh.  Inserts will be available immediately.

Testing:
- Created new custom cluster tests for object ownership

Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
---
M bin/create-test-configuration.sh
M bin/impala-config.sh
M common/thrift/JniCatalog.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/resources/mysql-hive-site.xml.template
M fe/src/test/resources/postgresql-hive-site.xml.template
M fe/src/test/resources/sentry-site.xml.template
A fe/src/test/resources/sentry-site_no_oo.xml.template
A fe/src/test/resources/sentry-site_oo_nogrant.xml.template
M testdata/bin/run-sentry-service.sh
A tests/authorization/test_owner_privileges.py
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_test_suite.py
31 files changed, 1,047 insertions(+), 41 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/11314/25
--
To view, visit http://gerrit.cloudera.org:8080/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 25
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 23:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/11314/24/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/24/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1017
PS24, Line 1017:* can be used to try adding the owner privilege again.
> might be worthwhile to state that while hms state will correctly reflect th
I've expanded the comment.  My understand is there is not a retry mechanism for 
the owner privileges.  I'll open a Jira against Sentry to somehow ensure 
synchronization.


http://gerrit.cloudera.org:8080/#/c/11314/24/fe/src/test/resources/sentry-site.xml.template
File fe/src/test/resources/sentry-site.xml.template:

http://gerrit.cloudera.org:8080/#/c/11314/24/fe/src/test/resources/sentry-site.xml.template@81
PS24, Line 81: 
> nit: remove the spacing
Done


http://gerrit.cloudera.org:8080/#/c/11314/23/tests/authorization/test_owner_privileges.py
File tests/authorization/test_owner_privileges.py:

http://gerrit.cloudera.org:8080/#/c/11314/23/tests/authorization/test_owner_privileges.py@156
PS23, Line 156:   def test_owner_privileges_with_grant(self, vector):
> create table if not exists 'foo' ... ?
Updated for tables.  Cannot update for database since the create runs as a 
different user.


http://gerrit.cloudera.org:8080/#/c/11314/24/tests/authorization/test_owner_privileges.py
File tests/authorization/test_owner_privileges.py:

http://gerrit.cloudera.org:8080/#/c/11314/24/tests/authorization/test_owner_privileges.py@34
PS24, Line 34: SENTRY_POLLING_FREQUENCY_S = 3
> flake8: F401 'tests.common.impala_test_suite.ImpalaTestSuite' imported but
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 23
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 07 Sep 2018 14:19:54 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-07 Thread Vuk Ercegovac (Code Review)

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 24:

(3 comments)

nits/clarifications, otherwise lgtm.

http://gerrit.cloudera.org:8080/#/c/11314/24/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/24/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1017
PS24, Line 1017:* the privilege, it will be removed on the next refresh. 
ALTER DATABASE SET OWNER
might be worthwhile to state that while hms state will correctly reflect the 
owner and the catalogd catalog will reflect the hms state, since sentry is 
updated async by hms, sentry's state may not reflect the state in hms. when an 
out-of-date syntry updates the catalogd, it will use sentries incorrect state. 
consequently, the state in catalogd may not always match the state in hms.

there will be some period that we could wait for sentry and hms to be in sync. 
perhaps we wait for their state to settle. if it doesn't, we roll-back the hms 
state? not sure if hms would give you the xact to effectively roll-back. I'm 
fine with not doing this now, but if its something that may be of interest, 
perhaps a todo is useful.


http://gerrit.cloudera.org:8080/#/c/11314/24/fe/src/test/resources/sentry-site.xml.template
File fe/src/test/resources/sentry-site.xml.template:

http://gerrit.cloudera.org:8080/#/c/11314/24/fe/src/test/resources/sentry-site.xml.template@81
PS24, Line 81:   
nit: remove the spacing


http://gerrit.cloudera.org:8080/#/c/11314/23/tests/authorization/test_owner_privileges.py
File tests/authorization/test_owner_privileges.py:

http://gerrit.cloudera.org:8080/#/c/11314/23/tests/authorization/test_owner_privileges.py@156
PS23, Line 156:
> Can't use that since I want to create the database after the cluster starts
create table if not exists 'foo' ... ?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 24
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 07 Sep 2018 06:18:18 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-06 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 24:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11314/24/tests/authorization/test_owner_privileges.py
File tests/authorization/test_owner_privileges.py:

http://gerrit.cloudera.org:8080/#/c/11314/24/tests/authorization/test_owner_privileges.py@34
PS24, Line 34: from tests.common.impala_test_suite import ImpalaTestSuite
flake8: F401 'tests.common.impala_test_suite.ImpalaTestSuite' imported but 
unused



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 24
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 07 Sep 2018 04:48:22 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-06 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 24:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/612/ : 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/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 24
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 07 Sep 2018 04:46:06 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has uploaded a new patch set (#24). ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..

IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

This patch adds calls to automatically create or remove owner
privileges in the catalog based on the statement.  This is similar to
the existing pattern where after privileges are granted in Sentry,
they are created in the catalog directly instead of pulled from
Sentry.

The sentry-site.xml and hive-site.xml template files have been updated
to enable usage of object ownership by default for easier development
with a secured impala.

When object ownership is enabled:
CREATE DATABASE will grant the user OWNER privileges to that database.
ALTER DATABASE SET OWNER will transfer the OWNER privileges to the
new owner.
DROP DATABASE will revoke the OWNER privileges from the owner.
This will apply to DATABASE, TABLE, and VIEW.

Example:
If ownership is enabled, when a table is created, the creator is the
owner, and Sentry will create owner privileges for the created table so
the user can continue working with it without waiting for Sentry
refresh.  Inserts will be available immediately.

Testing:
- Created new custom cluster tests for object ownership

Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
---
M bin/create-test-configuration.sh
M bin/impala-config.sh
M common/thrift/JniCatalog.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/resources/mysql-hive-site.xml.template
M fe/src/test/resources/postgresql-hive-site.xml.template
M fe/src/test/resources/sentry-site.xml.template
A fe/src/test/resources/sentry-site_no_oo.xml.template
A fe/src/test/resources/sentry-site_oo_nogrant.xml.template
M testdata/bin/run-sentry-service.sh
A tests/authorization/test_owner_privileges.py
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_test_suite.py
31 files changed, 1,044 insertions(+), 41 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/11314/24
--
To view, visit http://gerrit.cloudera.org:8080/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 24
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 23:

(16 comments)

http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java:

http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java@54
PS23, Line 54: authorization is enabled
> I got confused by what's supposed to happen if its enabled, but isObjectOwn
Done


http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1002
PS23, Line 1002: updateDatabasePrivileges(newDb.getName(), null, 
params.server_name, null, null,
> with so many args, it would be easier to read if the params were commented,
Done


http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1840
PS23, Line 1840: if (user == null) {
> looks like there's a race here (two operations see that owner doesn't exist
Done


http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2878
PS23, Line 2878:   .removePrivilege(privilege);
> is there a race here as well?
Duplicate removes should basically noop the second one.


http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3196
PS23, Line 3196: isObjectOwnershipEnabled
> after seeing this method, I'm wondering if this can be pulled up to analysi
Moved to updateDatabasePrivileges


http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/test/resources/sentry-site_no_oo.xml.template
File fe/src/test/resources/sentry-site_no_oo.xml.template:

http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/test/resources/sentry-site_no_oo.xml.template@76
PS23, Line 76: enable owner
> in this case, disable?
Done


http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/test/resources/sentry-site_oo_nogrant.xml.template
File fe/src/test/resources/sentry-site_oo_nogrant.xml.template:

http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/test/resources/sentry-site_oo_nogrant.xml.template@22
PS23, Line 22: sentry.service.security.mode
> add comments to highlight which of these settings pertain to "oo" and which
Done


http://gerrit.cloudera.org:8080/#/c/11314/23/tests/authorization/test_owner_privileges.py
File tests/authorization/test_owner_privileges.py:

http://gerrit.cloudera.org:8080/#/c/11314/23/tests/authorization/test_owner_privileges.py@70
PS23, Line 70: ImpalaTestSuite
> is this needed since CustomClusterTestSuite inherits already from ImpalaTes
Done


http://gerrit.cloudera.org:8080/#/c/11314/23/tests/authorization/test_owner_privileges.py@81
PS23, Line 81: Using user 'root
> I missed this.. where is 'root' explicitly used?
Adding a comment at the top to address.


http://gerrit.cloudera.org:8080/#/c/11314/23/tests/authorization/test_owner_privileges.py@87
PS23, Line 87: self.execute_query("create role owner_priv_test_ROOT")
> can things be simplified if you create a unique id per test in the test mat
Adding a comment at the top to address.  Basically need "root" because every 
system the tests run on should have a user root with group root.


http://gerrit.cloudera.org:8080/#/c/11314/23/tests/authorization/test_owner_privileges.py@109
PS23, Line 109: self.execute_query("create role owner_priv_admin")
  :
> why are these added when cleanup is supposed to remove things?
We create a role and grant it privileges so we can clean up otherwise we might 
not have access.  We don't want to rely on roles being in a state at the end of 
tests.


http://gerrit.cloudera.org:8080/#/c/11314/23/tests/authorization/test_owner_privileges.py@156
PS23, Line 156:   def test_owner_privileges_with_grant(self, vector):
> if you use the unique_database fixture here, all objects (db, table, view)
Can't use that since I want to create the database after the cluster starts up. 
 The unique_database creates the database before the test so owner privileges 
aren't created.  I tried adding 'if not exists' to the create and didn't work.


http://gerrit.cloudera.org:8080/#/c/11314/23/tests/authorization/test_owner_privileges.py@172
PS23, Line 172: self.root_impalad_client = self.create_impala_client()
  : s
> how do these two differ?
Adding a comment at the top to address.


http://gerrit.cloudera.org:8080/#/c/11314/23/tests/authorization/test_owner_privileges.py@248
PS23, Line 248: ser="root"
> maybe the comment about 'root' up on L81 is relevant here?

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-06 Thread Vuk Ercegovac (Code Review)

Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 23:

(16 comments)

http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java:

http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java@54
PS23, Line 54: authorization is enabled
I got confused by what's supposed to happen if its enabled, but 
isObjectOwnershipEnabled is false. What will be the end-state of the various 
systems under the four combinations here?


http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1002
PS23, Line 1002: updateDatabasePrivileges(newDb.getName(), null, 
params.server_name, null, null,
with so many args, it would be easier to read if the params were commented, 
e.g., (..., /*tableName*/null, ...), in particular for nulls and booleans.


http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1840
PS23, Line 1840: if (user == null) {
looks like there's a race here (two operations see that owner doesn't exist, 
one will go first and then the other will overwrite)... I see that addUser will 
overwrite so the perhaps the race is a no-op. however, it differs from the 
intent here so just wondering if anything unintended crops up here? for 
example, two operations for the same user will succeed so externally, will it 
look like the same user was added twice?

if there is an assumption about locking from the caller, pls state it.


http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2878
PS23, Line 2878:   .removePrivilege(privilege);
is there a race here as well?


http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3196
PS23, Line 3196: isObjectOwnershipEnabled
after seeing this method, I'm wondering if this can be pulled up to analysis? 
why let one of those alter commands get to this point when sentry object 
ownership is not enabled?

same question for the method below as well.

if the main explanation is that something is optional here-- is that spelled 
out somewhere?


http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/test/resources/sentry-site_no_oo.xml.template
File fe/src/test/resources/sentry-site_no_oo.xml.template:

http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/test/resources/sentry-site_no_oo.xml.template@76
PS23, Line 76: enable owner
in this case, disable?


http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/test/resources/sentry-site_oo_nogrant.xml.template
File fe/src/test/resources/sentry-site_oo_nogrant.xml.template:

http://gerrit.cloudera.org:8080/#/c/11314/23/fe/src/test/resources/sentry-site_oo_nogrant.xml.template@22
PS23, Line 22: sentry.service.security.mode
add comments to highlight which of these settings pertain to "oo" and which 
pertain to "nogrant".

pls add the same to the just "oo" sentry file as well. I have no idea which of 
these specify the intent of the file.


http://gerrit.cloudera.org:8080/#/c/11314/23/tests/authorization/test_owner_privileges.py
File tests/authorization/test_owner_privileges.py:

http://gerrit.cloudera.org:8080/#/c/11314/23/tests/authorization/test_owner_privileges.py@70
PS23, Line 70: ImpalaTestSuite
is this needed since CustomClusterTestSuite inherits already from 
ImpalaTestSuite?


http://gerrit.cloudera.org:8080/#/c/11314/23/tests/authorization/test_owner_privileges.py@81
PS23, Line 81: Using user 'root
I missed this.. where is 'root' explicitly used?


http://gerrit.cloudera.org:8080/#/c/11314/23/tests/authorization/test_owner_privileges.py@87
PS23, Line 87: self.execute_query("create role owner_priv_test_ROOT")
can things be simplified if you create a unique id per test in the test matrix? 
I see that all tests are marked to run serially, but in any event, I'm worried 
about leakage to other tests, or within this test.


http://gerrit.cloudera.org:8080/#/c/11314/23/tests/authorization/test_owner_privileges.py@109
PS23, Line 109: self.execute_query("create role owner_priv_admin")
  :
why are these added when cleanup is supposed to remove things?


http://gerrit.cloudera.org:8080/#/c/11314/23/tests/authorization/test_owner_privileges.py@156
PS23, Line 156:   def test_owner_privileges_with_grant(self, vector):
if you use the unique_database fixture here, all objects (db, table, view) will 
be cleaned up. The name can be

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-06 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 22:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11314/22/tests/authorization/test_owner_privileges.py
File tests/authorization/test_owner_privileges.py:

http://gerrit.cloudera.org:8080/#/c/11314/22/tests/authorization/test_owner_privileges.py@249
PS22, Line 249: "
flake8: E501 line too long (92 > 90 characters)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 22
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 06 Sep 2018 16:51:06 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-06 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 22:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/606/ : 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/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 22
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 06 Sep 2018 16:21:15 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

2018-09-06 Thread Impala Public Jenkins (Code Review)

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..


Patch Set 23:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/605/ : 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/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 23
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Thu, 06 Sep 2018 16:11:44 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

Adam Holley has uploaded a new patch set (#23). ( 
http://gerrit.cloudera.org:8080/11314 )

Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET 
OWNER
..

IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER

This patch adds calls to automatically create or remove owner
privileges in the catalog based on the statement.  This is similar to
the existing pattern where after privileges are granted in Sentry,
they are created in the catalog directly instead of pulled from
Sentry.

The sentry-site.xml and hive-site.xml template files have been updated
to enable usage of object ownership by default for easier development
with a secured impala.

When object ownership is enabled:
CREATE DATABASE will grant the user OWNER privileges to that database.
ALTER DATABASE SET OWNER will transfer the OWNER privileges to the
new owner.
DROP DATABASE will revoke the OWNER privileges from the owner.
This will apply to DATABASE, TABLE, and VIEW.

Example:
If ownership is enabled, when a table is created, the creator is the
owner, and Sentry will create owner privileges for the created table so
the user can continue working with it without waiting for Sentry
refresh.  Inserts will be available immediately.

Testing:
- Created new custom cluster tests for object ownership

Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
---
M bin/create-test-configuration.sh
M bin/impala-config.sh
M common/thrift/JniCatalog.thrift
M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M fe/src/test/resources/mysql-hive-site.xml.template
M fe/src/test/resources/postgresql-hive-site.xml.template
M fe/src/test/resources/sentry-site.xml.template
A fe/src/test/resources/sentry-site_no_oo.xml.template
A fe/src/test/resources/sentry-site_oo_nogrant.xml.template
M testdata/bin/run-sentry-service.sh
A tests/authorization/test_owner_privileges.py
M tests/common/custom_cluster_test_suite.py
M tests/common/impala_test_suite.py
31 files changed, 1,038 insertions(+), 41 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/11314/23
--
To view, visit http://gerrit.cloudera.org:8080/11314
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0
Gerrit-Change-Number: 11314
Gerrit-PatchSet: 23
Gerrit-Owner: Adam Holley 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER