[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 15: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 15 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Fri, 04 Jan 2019 07:22:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. IMPALA-7795: Implement REFRESH AUTHORIZATION statement This patch implements REFRESH AUTHORIZATION statement to explicitly refresh authorization metadata. This statement is useful to force Impala to refresh its authorization metadata when there is an external update to authorization metadata without having to wait for the Sentry polling or call INVALIDATE METADATA. Some tests were updated to use REFRESH AUTHORIZATION instead of INVALIDATE METADATA to make the tests run faster. Syntax: REFRESH AUTHORIZATION (authorization must be enabled to execute this statement) Testing: - Added new FE tests - Added new E2E authorization tests - Ran all FE tests - Ran all E2E authorization tests Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Reviewed-on: http://gerrit.cloudera.org:8080/11888 Reviewed-by: Fredy Wijaya Tested-by: Impala Public Jenkins --- M common/thrift/CatalogService.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.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/util/SentryProxy.java M fe/src/main/jflex/sql-scanner.flex M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/util/SentryProxyTest.java M tests/authorization/test_authorization.py M tests/authorization/test_grant_revoke.py M tests/authorization/test_owner_privileges.py M tests/common/sentry_cache_test_suite.py 18 files changed, 525 insertions(+), 223 deletions(-) Approvals: Fredy Wijaya: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 16 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 15: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3611/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 15 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Fri, 04 Jan 2019 03:24:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 15: Code-Review+2 Carry Bharath's +2. -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 15 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Fri, 04 Jan 2019 03:23:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 15: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1709/ : 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/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 15 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Fri, 04 Jan 2019 03:08:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has uploaded a new patch set (#15). ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. IMPALA-7795: Implement REFRESH AUTHORIZATION statement This patch implements REFRESH AUTHORIZATION statement to explicitly refresh authorization metadata. This statement is useful to force Impala to refresh its authorization metadata when there is an external update to authorization metadata without having to wait for the Sentry polling or call INVALIDATE METADATA. Some tests were updated to use REFRESH AUTHORIZATION instead of INVALIDATE METADATA to make the tests run faster. Syntax: REFRESH AUTHORIZATION (authorization must be enabled to execute this statement) Testing: - Added new FE tests - Added new E2E authorization tests - Ran all FE tests - Ran all E2E authorization tests Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c --- M common/thrift/CatalogService.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.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/util/SentryProxy.java M fe/src/main/jflex/sql-scanner.flex M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/util/SentryProxyTest.java M tests/authorization/test_authorization.py M tests/authorization/test_grant_revoke.py M tests/authorization/test_owner_privileges.py M tests/common/sentry_cache_test_suite.py 18 files changed, 525 insertions(+), 223 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/11888/15 -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 15 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/11888 )
Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement
..
Patch Set 15:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/11888/14/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/11888/14/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1162
PS14, Line 1162: catalog object
> nit: ..objects added.. (below too)
Done
http://gerrit.cloudera.org:8080/#/c/11888/14/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/11888/14/fe/src/main/java/org/apache/impala/util/SentryProxy.java@75
PS14, Line 75: catalog
> nit: catalog
Done
http://gerrit.cloudera.org:8080/#/c/11888/14/fe/src/main/java/org/apache/impala/util/SentryProxy.java@261
PS14, Line 261: authzDelta.getRemovedCatalogObjects().add(
> Log something here for debugging?
Done
http://gerrit.cloudera.org:8080/#/c/11888/14/tests/authorization/test_authorization.py
File tests/authorization/test_authorization.py:
http://gerrit.cloudera.org:8080/#/c/11888/14/tests/authorization/test_authorization.py@524
PS14, Line 524: y adding and removing r
> rephrase?
Done
http://gerrit.cloudera.org:8080/#/c/11888/14/tests/authorization/test_authorization.py@529
PS14, Line 529:
> nit: I think you can directly do:
Done
http://gerrit.cloudera.org:8080/#/c/11888/14/tests/authorization/test_authorization.py@550
PS14, Line 550:
> add a docstring
Done
http://gerrit.cloudera.org:8080/#/c/11888/14/tests/authorization/test_authorization.py@591
PS14, Line 591: self.role_cleanup("%s_%s" % (unique_role, suffix))
> doc string
Done
http://gerrit.cloudera.org:8080/#/c/11888/14/tests/authorization/test_authorization.py@606
PS14, Line 606: result = self.execute_query_expect_success(self.client,
"show grant role %s" %
:unique_role)
> nit: should we loop this for all the clients?
Done
--
To view, visit http://gerrit.cloudera.org:8080/11888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c
Gerrit-Change-Number: 11888
Gerrit-PatchSet: 15
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Comment-Date: Fri, 04 Jan 2019 02:38:27 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/11888 )
Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement
..
Patch Set 14: Code-Review+2
(8 comments)
just a bunch of nits, lgtm otherwise.
http://gerrit.cloudera.org:8080/#/c/11888/14/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/11888/14/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1162
PS14, Line 1162: catalogs added
nit: ..objects added.. (below too)
http://gerrit.cloudera.org:8080/#/c/11888/14/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/11888/14/fe/src/main/java/org/apache/impala/util/SentryProxy.java@75
PS14, Line 75: catalogs
nit: catalog
http://gerrit.cloudera.org:8080/#/c/11888/14/fe/src/main/java/org/apache/impala/util/SentryProxy.java@261
PS14, Line 261: }
Log something here for debugging?
Added x roles, removed y roles ..
http://gerrit.cloudera.org:8080/#/c/11888/14/tests/authorization/test_authorization.py
File tests/authorization/test_authorization.py:
http://gerrit.cloudera.org:8080/#/c/11888/14/tests/authorization/test_authorization.py@524
PS14, Line 524: The Sentry long polling
rephrase?
http://gerrit.cloudera.org:8080/#/c/11888/14/tests/authorization/test_authorization.py@529
PS14, Line 529: 1 if sync_ddl else 0
nit: I think you can directly do:
for sync_ddl in [0, 1]:
opts = {'sync_ddl': sync_ddl}
if sync_ddl:
http://gerrit.cloudera.org:8080/#/c/11888/14/tests/authorization/test_authorization.py@550
PS14, Line 550:clients):
add a docstring
http://gerrit.cloudera.org:8080/#/c/11888/14/tests/authorization/test_authorization.py@591
PS14, Line 591: clients):
doc string
http://gerrit.cloudera.org:8080/#/c/11888/14/tests/authorization/test_authorization.py@606
PS14, Line 606: self.execute_query_expect_failure(self.client,
: "select * from
functional.alltypes limit 1")
nit: should we loop this for all the clients?
--
To view, visit http://gerrit.cloudera.org:8080/11888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c
Gerrit-Change-Number: 11888
Gerrit-PatchSet: 14
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Comment-Date: Fri, 04 Jan 2019 00:49:07 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 14: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1703/ : 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/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 14 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 03 Jan 2019 23:02:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/11888 )
Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement
..
Patch Set 14:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/11888/13/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java:
http://gerrit.cloudera.org:8080/#/c/11888/13/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@110
PS13, Line 110: return new ResetMetadataStmt(Action.REFRESH_AUTHORIZATION,
/*db*/ null,
> nit:
Done
http://gerrit.cloudera.org:8080/#/c/11888/13/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/11888/13/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3457
PS13, Line 3457: else {
: // Invalidate the entire catalog if no table name is
provided.
:
> Is this needed? Doesn't refreshAuthorization() take care of it?
You're right. Those authorization objects are added into the delete logs by
CatalogServiceCatalog. Done.
http://gerrit.cloudera.org:8080/#/c/11888/13/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/11888/13/fe/src/main/java/org/apache/impala/util/SentryProxy.java@74
PS13, Line 74: /**
> Class comment.
Done
http://gerrit.cloudera.org:8080/#/c/11888/13/fe/src/main/java/org/apache/impala/util/SentryProxy.java@75
PS13, Line 75:* This class keeps track of catalogs objects added and
removed.
> Not sure if this is the right name. Isn't this tracking the Delta, probably
Done
http://gerrit.cloudera.org:8080/#/c/11888/13/fe/src/main/java/org/apache/impala/util/SentryProxy.java@188
PS13, Line 188: y {
> Like mentioned below, I think we can get rid of this and handle everything
Done. I agree it feels weird to deal with deletion of catalog objects in this
method. It's probably a remnant of old code. I updated the code to remove the
catalog users and roles in refreshRolePrivileges and refreshUserPrivileges
instead.
http://gerrit.cloudera.org:8080/#/c/11888/13/fe/src/main/java/org/apache/impala/util/SentryProxy.java@261
PS13, Line 261: }
> It is weird that this method is not updating the removed objects.Instead it
Done
http://gerrit.cloudera.org:8080/#/c/11888/13/tests/authorization/test_authorization.py
File tests/authorization/test_authorization.py:
http://gerrit.cloudera.org:8080/#/c/11888/13/tests/authorization/test_authorization.py@517
PS13, Line 517: @CustomClusterTestSuite.with_args(
> Should we add some test coverage for sync_ddl? I'd assume that is a common
Done
--
To view, visit http://gerrit.cloudera.org:8080/11888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c
Gerrit-Change-Number: 11888
Gerrit-PatchSet: 14
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Comment-Date: Thu, 03 Jan 2019 22:31:47 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has uploaded a new patch set (#14). ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. IMPALA-7795: Implement REFRESH AUTHORIZATION statement This patch implements REFRESH AUTHORIZATION statement to explicitly refresh authorization metadata. This statement is useful to force Impala to refresh its authorization metadata when there is an external update to authorization metadata without having to wait for the Sentry polling or call INVALIDATE METADATA. Some tests were updated to use REFRESH AUTHORIZATION instead of INVALIDATE METADATA to make the tests run faster. Syntax: REFRESH AUTHORIZATION (authorization must be enabled to execute this statement) Testing: - Added new FE tests - Added new E2E authorization tests - Ran all FE tests - Ran all E2E authorization tests Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c --- M common/thrift/CatalogService.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.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/util/SentryProxy.java M fe/src/main/jflex/sql-scanner.flex M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/util/SentryProxyTest.java M tests/authorization/test_authorization.py M tests/authorization/test_grant_revoke.py M tests/authorization/test_owner_privileges.py M tests/common/sentry_cache_test_suite.py 18 files changed, 511 insertions(+), 223 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/11888/14 -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 14 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/11888 )
Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement
..
Patch Set 13:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/11888/13/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java:
http://gerrit.cloudera.org:8080/#/c/11888/13/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@110
PS13, Line 110: return new ResetMetadataStmt(Action.REFRESH_FUNCTIONS, db,
/*table*/ null,
nit:
return new Reset..Stmt(Action.REFRESH_FUNCTIONS,
Preconditions.checkNotNull(db), null, null)
(multiple places).
http://gerrit.cloudera.org:8080/#/c/11888/13/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/11888/13/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3457
PS13, Line 3457: for (TCatalogObject obj: removed) {
: catalog_.getDeleteLog().addRemovedObject(obj);
:
Is this needed? Doesn't refreshAuthorization() take care of it?
http://gerrit.cloudera.org:8080/#/c/11888/13/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/11888/13/fe/src/main/java/org/apache/impala/util/SentryProxy.java@74
PS13, Line 74: @VisibleForTesting
Class comment.
http://gerrit.cloudera.org:8080/#/c/11888/13/fe/src/main/java/org/apache/impala/util/SentryProxy.java@75
PS13, Line 75: protected static class AuthorizationCatalog {
Not sure if this is the right name. Isn't this tracking the Delta, probably
AuthorizationDelta or something along those lines?
http://gerrit.cloudera.org:8080/#/c/11888/13/fe/src/main/java/org/apache/impala/util/SentryProxy.java@188
PS13, Line 188: rolesToRemove
Like mentioned below, I think we can get rid of this and handle everything
using AuthzDelta...
http://gerrit.cloudera.org:8080/#/c/11888/13/fe/src/main/java/org/apache/impala/util/SentryProxy.java@261
PS13, Line 261:
authzCatalog.getAddedCatalogObjects().add(role.toTCatalogObject());
It is weird that this method is not updating the removed objects.Instead it is
doing it above in the caller.
Same with Users.
http://gerrit.cloudera.org:8080/#/c/11888/13/tests/authorization/test_authorization.py
File tests/authorization/test_authorization.py:
http://gerrit.cloudera.org:8080/#/c/11888/13/tests/authorization/test_authorization.py@517
PS13, Line 517: @CustomClusterTestSuite.with_args(
Should we add some test coverage for sync_ddl? I'd assume that is a common use
case from customers.
IMO following combinations should be tested.
sync_ddl in {true, false}
- Add X roles drop Y roles
- Add X roles, drop nothing
- Drop Y roles, add nothing
(if sync_ddl is true, make sure the changes are propagated in a different
coordinator)
--
To view, visit http://gerrit.cloudera.org:8080/11888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c
Gerrit-Change-Number: 11888
Gerrit-PatchSet: 13
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Comment-Date: Fri, 21 Dec 2018 20:34:30 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 13: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 13 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Fri, 14 Dec 2018 21:39:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 13: Thanks much for making the changes: the code is now much simpler and easier to read for newbies like me. +1 (non-binding) on the Java side. -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 13 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Fri, 14 Dec 2018 21:12:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 13: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1611/ : 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/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 13 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Fri, 14 Dec 2018 19:59:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/11888 )
Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement
..
Patch Set 13:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java:
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@56
PS12, Line 56:
> Nit: less cluttered to put enums, nested classes at the top of the class, f
Done
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@122
PS12, Line 122: protected Action getAction() { return action_; }
> Nit: include "protected" keyword. It is the default, yes, but convenient fo
Done
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@141
PS12, Line 141: if (action_.isRefresh()) {
> action_.isRefresh() -- use method, not private field
Done
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@182
PS12, Line 182: throw new IllegalStateException("Invalid reset metadata
action: " + action_);
> Nit: This is kind of overkill. Simply throw IllegalStateException("Invalid.
Done
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@190
PS12, Line 190: case REFRESH_AUTHORIZATION:
> action_.isRefresh()
Done
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@191
PS12, Line 191: result.append("REFRESH AUTHORIZATION");
> Actually, this whole block can be simpler:
Originally I was following the old style, but I agree I don't think there's any
point saving few words with more complicated if/else/switch. Done.
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@204
PS12, Line 204: result.append("INVALIDATE METADATA");
> Nit: .append(" ").append(partitionSpec...
Done
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@207
PS12, Line 207: result.append("INVALIDATE METADATA
").append(tableName_.toSql());
> Nit: see above
Done
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@216
PS12, Line 216: TResetMetadataRequest params = new TResetMetadataRequest();
> Nit: result.append("INVALIDATE METADATA").append(" ") --> result.append("IN
Good catch! Done.
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@219
PS12, Line 219: params.setTable_name(new TTableName(tableName_.getDb(),
tableName_.getTbl()));
> Nit: see above
Done
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@228
PS12, Line 228: }
> Use method
Done
http://gerrit.cloudera.org:8080/#/c/11888/12/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/11888/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3458
PS12, Line 3458: catalog_.getDeleteLog().addRemovedObject(obj);
> addRemovedObject? Does this really add to the catalog an object that was re
Yup, that's what addRemovedObject means. I'd rather not rename anything in the
existing catalog code for now.
No, deletion is handled differently in the catalog, hence the need to have
getDeleteLog().
http://gerrit.cloudera.org:8080/#/c/11888/12/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/11888/12/fe/src/main/java/org/apache/impala/util/SentryProxy.java@265
PS12, Line 265: refreshPrivilegesInCatalog(catalog, resetVersions,
sentryRole.getRoleName(), role,
> The arguments are getting complex. And, it is awkward to return values via
It's a bit trickier since the whole thing runs in a thread so there really
isn't a good way to use a return type unless we modify the whole thing to
return a Future, but I think that's a better task for another CR. However, I do
agree that the number of arguments are getting unwieldy. In the next PS, I
introduced a private class AuthorizationCatalog to capture the added/removed
catalog objects. Let me know what you think.
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/util/SentryProxy.java@305
PS12, Line 305: user, allUsersPrivileges, authzCatalog);
> This one is also getting cluttered.
Done
--
To view, visit
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has uploaded a new patch set (#13). ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. IMPALA-7795: Implement REFRESH AUTHORIZATION statement This patch implements REFRESH AUTHORIZATION statement to explicitly refresh authorization metadata. This statement is useful to force Impala to refresh its authorization metadata when there is an external update to authorization metadata without having to wait for the Sentry polling or call INVALIDATE METADATA. Some tests were updated to use REFRESH AUTHORIZATION instead of INVALIDATE METADATA to make the tests run faster. Syntax: REFRESH AUTHORIZATION (authorization must be enabled to execute this statement) Testing: - Added new FE tests - Added new E2E authorization tests - Ran all FE tests - Ran all E2E authorization tests Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c --- M common/thrift/CatalogService.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.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/util/SentryProxy.java M fe/src/main/jflex/sql-scanner.flex M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/util/SentryProxyTest.java M tests/authorization/test_authorization.py M tests/authorization/test_grant_revoke.py M tests/authorization/test_owner_privileges.py M tests/common/sentry_cache_test_suite.py 18 files changed, 441 insertions(+), 203 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/11888/13 -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 13 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Paul Rogers has posted comments on this change. (
http://gerrit.cloudera.org:8080/11888 )
Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement
..
Patch Set 12:
(14 comments)
Coming along nicely. A few more comments.
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java:
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@56
PS12, Line 56: public enum Action {
Nit: less cluttered to put enums, nested classes at the top of the class,
followed by statics, then non-static members
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@122
PS12, Line 122: Action getAction() { return action_; }
Nit: include "protected" keyword. It is the default, yes, but convenient for
readers to add it.
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@141
PS12, Line 141: if (action_.isRefresh_) {
action_.isRefresh() -- use method, not private field
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@182
PS12, Line 182: Preconditions.checkState(false, "Invalid reset metadata
action: " + action_);
Nit: This is kind of overkill. Simply throw IllegalStateException("Invalid...
is more direct. If you look in Preconditions.checkState, that's all it will do.
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@190
PS12, Line 190: if (action_.isRefresh_) {
action_.isRefresh()
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@191
PS12, Line 191: result.append("REFRESH");
Actually, this whole block can be simpler:
if (action_.isRefresh()) result.append("REFRESH ");
Then, the two switch statements can be combined into one.
Or, like was done with INVALIDATE, just include the REFRESH keyword in the
individual case.
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@204
PS12, Line 204: .append(" " + partitionSpec_.toSql(options));
Nit: .append(" ").append(partitionSpec...
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@207
PS12, Line 207: Preconditions.checkState(false, "Invalid reset
metadata action: " + action_);
Nit: see above
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@216
PS12, Line 216: result.append("INVALIDATE METADATA").append("
").append(tableName_);
Nit: result.append("INVALIDATE METADATA").append(" ") -->
result.append("INVALIDATE METADATA ")
Also, ToSqlUtils.getIdentSql(tableName_) . Handles the case where table name is
a keyword, say, "select".
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@219
PS12, Line 219: Preconditions.checkState(false, "Invalid reset
metadata action: " + action_);
Nit: see above
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@228
PS12, Line 228: params.setIs_refresh(action_.isRefresh_);
Use method
http://gerrit.cloudera.org:8080/#/c/11888/12/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/11888/12/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3458
PS12, Line 3458: catalog_.getDeleteLog().addRemovedObject(obj);
addRemovedObject? Does this really add to the catalog an object that was
removed? Better name?
Also, anything we need to do with the added list? If not, maybe include a
comment to explain why?
http://gerrit.cloudera.org:8080/#/c/11888/12/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/11888/12/fe/src/main/java/org/apache/impala/util/SentryProxy.java@265
PS12, Line 265: List added, List removed)
The arguments are getting complex. And, it is awkward to return values via
arguments. Consider defining a RefreshAction class of some sort that takes the
various options, is passed in here, and can return the results.
Looks like we already have something similar above with the PolicyReader class.
Can we either use that, or create another class like that mentioned above?
http://gerrit.cloudera.org:8080/#/c/11888/12/fe/src/main/java/org/apache/impala/util/SentryProxy.java@305
PS12, Line 305: List added, List removed)
This one is
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 12: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1592/ : 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/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 12 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 13 Dec 2018 00:21:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/11888 )
Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement
..
Patch Set 12:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/11888/11/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java:
http://gerrit.cloudera.org:8080/#/c/11888/11/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@138
PS11, Line 138: Preconditions.checkNotNull(tableName_);
> Preconditions.checkNotNull(tableName_) between L137-138
Done
http://gerrit.cloudera.org:8080/#/c/11888/8/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/11888/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1140
PS8, Line 1140: blic void refreshAuthorizat
> Can we reverse it then. if (sentryProxy == null) return;
Done.
http://gerrit.cloudera.org:8080/#/c/11888/8/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/11888/8/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3451
PS8, Line 3451: List added = new ArrayList<>();
> Actually thinking a bit more about this, I still am not convinced. I think
Thanks for the explanation, that makes sense. I have updated the code to keep
track of list of catalog objects added and removed.
http://gerrit.cloudera.org:8080/#/c/11888/8/tests/authorization/test_authorization.py
File tests/authorization/test_authorization.py:
http://gerrit.cloudera.org:8080/#/c/11888/8/tests/authorization/test_authorization.py@546
PS8, Line 546:
> Can we also do execute_query_expect_failure(select from functional.alltypes
Done
http://gerrit.cloudera.org:8080/#/c/11888/8/tests/authorization/test_authorization.py@555
PS8, Line 555: assert any("select" in x for x in result.data)
> execute_query_expect_success()
Done
http://gerrit.cloudera.org:8080/#/c/11888/8/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:
http://gerrit.cloudera.org:8080/#/c/11888/8/tests/authorization/test_grant_revoke.py@211
PS8, Line 211: update.
: """
: db_name = "test_role_privilege_case_x_" + get_random_id(5)
: db_name_upper_case = "TEST_ROLE_PRIVILEGE_CASE_Y_" +
get_random_id(5).upper()
: db_name_mixed_case = "TesT_Role_PRIVIlege_case_z" +
get_random_id(5)
: role_name = "test_role_" + get_random_id(5)
: try:
: self.client.execute("create role {0}".format(role_name))
: self.client.execute("grant all on server to
{0}".format(role_name))
: self.client.execute("grant role {0} to group
`{1}`".format(role_name,
: grp.getgrnam(getuser()).gr_name))
:
: self.client.execute("create database " + db_name)
: self.client.execute("create database " +
db_name_upper_case)
: self.client.execute("create database " +
db_name_mixed_case)
: self.client.execute(
: "create table if not exists {0}.test1(i
int)".format(db_name))
: self.client.execute("create table if not exists
{0}.TEST2(i int)".format(db_name))
: self.client.execute("create table if not exists
{0}.Test3(i int)".format(db_name))
:
: self.client.execute(
: "grant select on table {0}.test1 to
{1}".format(db_name, role_name))
: self.client.execute(
: "grant select on table {0}.TEST2 to
{1}".format(db_name, role_name))
: self.client.execute(
: "grant select on table {0}.TesT3 to
{1}".format(db_name, role_name))
: self.client.execute("grant all on database {0} to
{1}".format(db_name, role_name))
: self.client.execute(
: "grant all on database {0} to
{1}".format(db_name_upper_case, role_name))
: self.client.execute(
: "grant all on database {0} to
{1}".format(db_name_mixed_case, role_name))
: result = self.client.execute("show grant role
{0}".format(role_name))
: assert any('test1' in x for x in result.data)
: assert any('test2' in x for x in result.data)
: assert any('test3' in x for x in result.data)
> can we fold this into test_sentry_refresh?
Yeah this actually looks very similar to test_refresh_authorization. I don't
know why I put this test here
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. IMPALA-7795: Implement REFRESH AUTHORIZATION statement This patch implements REFRESH AUTHORIZATION statement to explicitly refresh authorization metadata. This statement is useful to force Impala to refresh its authorization metadata when there is an external update to authorization metadata without having to wait for the Sentry polling or call INVALIDATE METADATA. Some tests were updated to use REFRESH AUTHORIZATION instead of INVALIDATE METADATA to make the tests run faster. Syntax: REFRESH AUTHORIZATION (authorization must be enabled to execute this statement) Testing: - Added new FE tests - Added new E2E authorization tests - Ran all FE tests - Ran all E2E authorization tests Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c --- M common/thrift/CatalogService.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.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/util/SentryProxy.java M fe/src/main/jflex/sql-scanner.flex M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/util/SentryProxyTest.java M tests/authorization/test_authorization.py M tests/authorization/test_grant_revoke.py M tests/authorization/test_owner_privileges.py M tests/common/sentry_cache_test_suite.py 18 files changed, 431 insertions(+), 197 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/11888/12 -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 12 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/11888 )
Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement
..
Patch Set 11:
(7 comments)
The code looks fine to me. I still have some concerns around correctness.
http://gerrit.cloudera.org:8080/#/c/11888/11/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java:
http://gerrit.cloudera.org:8080/#/c/11888/11/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@138
PS11, Line 138: String dbName = analyzer.getTargetDbName(tableName_);
Preconditions.checkNotNull(tableName_) between L137-138
http://gerrit.cloudera.org:8080/#/c/11888/8/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/11888/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1140
PS8, Line 1140: blic void refreshAuthorizat
> Actually we set sentryProxy_ to null in https://github.com/apache/impala/bl
Can we reverse it then. if (sentryProxy == null) return;
http://gerrit.cloudera.org:8080/#/c/11888/8/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/11888/8/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3451
PS8, Line 3451: catalog_.refreshAuthorization(false);
> We actually increment the catalog version in L3452. I did a quick experimen
Actually thinking a bit more about this, I still am not convinced. I think
there are a few subtle problems here.
1. catalog_.getCatalogVersion() could potentially return a value > what we need
here. Reason being that since you are not locking the versions globally, there
could potentially be concurrent operations that increment versions between
L3451 and L3452.
2. I still am not convinced by the correctness of this. If you see the
coordinator side synchronous wait loop for this, it looks like follows,
Status ImpalaServer::ProcessCatalogUpdateResult(
const TCatalogUpdateResult& catalog_update_result, bool
wait_for_all_subscribers) {
const TUniqueId& catalog_service_id =
catalog_update_result.catalog_service_id;
if (!catalog_update_result.__isset.updated_catalog_objects &&
!catalog_update_result.__isset.removed_catalog_objects) {
// Operation with no result set. Use the version specified in
// 'catalog_update_result' to determine when the effects of this operation
// have been applied to the local catalog cache.
if (catalog_update_result.is_invalidate) {
WaitForMinCatalogUpdate(catalog_update_result.version,
catalog_service_id);
} else {
WaitForCatalogUpdate(catalog_update_result.version, catalog_service_id);
}
In this case since you are setting the version, you'd block on
WaitForCatalogUpdate(catalog_update_result.version, catalog_service_id).
However, since SS does not guarantee broadcast in the order of versions, there
is a chance that the loop in WaitForCatalogUpdate() might break early.
If you see invalidate metadata (which uses a similar logic), you see it calls
WaitForMinCatalogUpdate(). Notice the *Min* part.
3. I'm afraid that if we take the invalidate approach, this becomes
incompatible with Catalog V2 (which does not include an easy way of finding min
versions on the coordinator).
Please let me know if I'm wrong in my understanding.
http://gerrit.cloudera.org:8080/#/c/11888/8/tests/authorization/test_authorization.py
File tests/authorization/test_authorization.py:
http://gerrit.cloudera.org:8080/#/c/11888/8/tests/authorization/test_authorization.py@546
PS8, Line 546:
Can we also do execute_query_expect_failure(select from functional.alltypes)
here.
http://gerrit.cloudera.org:8080/#/c/11888/8/tests/authorization/test_authorization.py@555
PS8, Line 555: finally:
execute_query_expect_success()
http://gerrit.cloudera.org:8080/#/c/11888/8/tests/authorization/test_grant_revoke.py
File tests/authorization/test_grant_revoke.py:
http://gerrit.cloudera.org:8080/#/c/11888/8/tests/authorization/test_grant_revoke.py@211
PS8, Line 211: """Tests that explicit grant/revoke within Impala and external
privilege update should
: produce correct privileges in Impala after a Sentry
refresh."""
: group_name = grp.getgrnam(getuser()).gr_name
: try:
: self.client.execute("create role %s" % unique_role)
: self.client.execute("grant role %s to group `%s`" %
(unique_role, group_name))
: # Add select and insert into the catalog.
: self.client.execute("grant refresh on server to %s" %
unique_role)
:
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Paul Rogers has posted comments on this change. (
http://gerrit.cloudera.org:8080/11888 )
Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement
..
Patch Set 11: Code-Review+1
(1 comment)
LGTM. The parser/AST changes all look good, especially after moving to the
action enum. Can't speak to the Python tests or actual authorization code.
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/jflex/sql-scanner.flex
File fe/src/main/jflex/sql-scanner.flex:
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/jflex/sql-scanner.flex@78
PS9, Line 78: keywordMap.put("authorization",
SqlParserSymbols.KW_AUTHORIZATION);
> I believe authorization is a reserved keyword as specified in L321 of the o
Ah, missed the list of reserved words which are not keywords. Then there is no
regression risk, and the proposed syntax is fine.
--
To view, visit http://gerrit.cloudera.org:8080/11888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c
Gerrit-Change-Number: 11888
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Comment-Date: Fri, 07 Dec 2018 01:12:12 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1552/ : 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/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 11 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 06 Dec 2018 02:39:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/11888 )
Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement
..
Patch Set 11:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java:
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@59
PS9, Line 59: REFRESH_TABLE(true),
> We now have two flags: isRefresh, and authorization. Are these disjoint? On
Yeah I agree this class is somewhat convoluted. I like your suggestion. Done.
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@111
PS9, Line 111: /*partition*/ null);
> The logic here is getting hard to follow. Would be easier using a case stat
Done
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@160
PS9, Line 160: }
> Same comment as above.
Done
http://gerrit.cloudera.org:8080/#/c/11888/9/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/11888/9/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1140
PS9, Line 1140: public void refreshAuthorization(boolean resetVersions)
throws CatalogException {
> if (sentryProxy == null) return;
Done
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1140
PS9, Line 1140: public void refreshAuthorization(boolean resetVersions)
throws CatalogException {
> if (sentryProxy == null) return;
Done
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/jflex/sql-scanner.flex
File fe/src/main/jflex/sql-scanner.flex:
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/jflex/sql-scanner.flex@78
PS9, Line 78: keywordMap.put("authorization",
SqlParserSymbols.KW_AUTHORIZATION);
> A persistent concern when adding keywords is that some queries will now fai
I believe authorization is a reserved keyword as specified in L321 of the
original code. REFRESH ROLE and REFRESH GRANT are too specific and not generic
enough we want the statement to basically refresh anything related to
authorization metadata.
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java:
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java@331
PS9, Line 331: assertAction.accept(AnalyzesOk("invalidate metadata"),
> Ideally, here or elsewhere, we'd inspect the resulting statement to ensure
Good idea. Done.
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
File fe/src/test/java/org/apache/impala/common/FrontendTestBase.java:
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java@451
PS9, Line 451:* Creates an authorization config for creating an
AnalysisContext with
> Maybe explain this a bit: what is covered in the file? What does this enabl
Done
http://gerrit.cloudera.org:8080/#/c/11888/9/tests/authorization/test_authorization.py
File tests/authorization/test_authorization.py:
http://gerrit.cloudera.org:8080/#/c/11888/9/tests/authorization/test_authorization.py@547
PS9, Line 547: self.execute_query_expect_success(self.client, "refresh
authorization")
> Not specific to this test, but what do we return from this statement? Shoul
Good idea. Done.
--
To view, visit http://gerrit.cloudera.org:8080/11888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c
Gerrit-Change-Number: 11888
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Comment-Date: Thu, 06 Dec 2018 01:59:54 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. IMPALA-7795: Implement REFRESH AUTHORIZATION statement This patch implements REFRESH AUTHORIZATION statement to explicitly refresh authorization metadata. This statement is useful to force Impala to refresh its authorization metadata when there is an external update to authorization metadata without having to wait for the Sentry polling or call INVALIDATE METADATA. Some tests were updated to use REFRESH AUTHORIZATION instead of INVALIDATE METADATA to make the tests run faster. Syntax: REFRESH AUTHORIZATION (authorization must be enabled to execute this statement) Testing: - Added new FE tests - Added new E2E authorization tests - Ran all FE tests - Ran all E2E authorization tests Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c --- M common/thrift/CatalogService.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.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/jflex/sql-scanner.flex M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M tests/authorization/test_authorization.py M tests/authorization/test_grant_revoke.py M tests/authorization/test_owner_privileges.py M tests/common/sentry_cache_test_suite.py 16 files changed, 417 insertions(+), 170 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/11888/11 -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 11 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Paul Rogers has posted comments on this change. (
http://gerrit.cloudera.org:8080/11888 )
Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement
..
Patch Set 9:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java:
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@59
PS9, Line 59: PartitionSpec partitionSpec, String db, boolean
authorization) {
We now have two flags: isRefresh, and authorization. Are these disjoint? One is
true or the other? And, how to they relate to partitionSpec?
Should we define an enum to identify the meaning of the statement rather than
parsing it from flags?
enum Action{ REFRESH_AUTH, REFRESH_TABLE, REFRESH_ALL, ... };
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@111
PS9, Line 111: if (authorization_) {
The logic here is getting hard to follow. Would be easier using a case
statement with that enum mentioned above.
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@160
PS9, Line 160: result.append(" AUTHORIZATION");
Same comment as above.
http://gerrit.cloudera.org:8080/#/c/11888/9/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/11888/9/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1140
PS9, Line 1140: if (sentryProxy_ != null) {
if (sentryProxy == null) return;
Then, outdent the try/catch block. Result is a bit easier to read.
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/jflex/sql-scanner.flex
File fe/src/main/jflex/sql-scanner.flex:
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/main/jflex/sql-scanner.flex@78
PS9, Line 78: keywordMap.put("authorization",
SqlParserSymbols.KW_AUTHORIZATION);
A persistent concern when adding keywords is that some queries will now fail:
SELECT authorization FROM mySecurityTable
Is there some existing keyword that could be used instead, even if not ideal?
Maybe REFRESH ROLE METADATA? REFRESH GRANT METADATA?
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java:
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java@331
PS9, Line 331: AnalyzesOk("refresh authorization",
createAnalysisCtx(createAuthorizationConfig()));
Ideally, here or elsewhere, we'd inspect the resulting statement to ensure all
the knick-knacks are set as you expect: flags, fields, etc. Ensure that
predicates fire as you expect: is/is not a table action is an authorization
action, etc.
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
File fe/src/test/java/org/apache/impala/common/FrontendTestBase.java:
http://gerrit.cloudera.org:8080/#/c/11888/9/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java@451
PS9, Line 451: AuthorizationConfig authzConfig =
AuthorizationConfig.createHadoopGroupAuthConfig(
Maybe explain this a bit: what is covered in the file? What does this enabled?
When should we use this form?
http://gerrit.cloudera.org:8080/#/c/11888/9/tests/authorization/test_authorization.py
File tests/authorization/test_authorization.py:
http://gerrit.cloudera.org:8080/#/c/11888/9/tests/authorization/test_authorization.py@547
PS9, Line 547: self.client.execute("refresh authorization")
Not specific to this test, but what do we return from this statement? Should we
return a warning or message if no auth is configured? A success message or code
if successful?
--
To view, visit http://gerrit.cloudera.org:8080/11888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c
Gerrit-Change-Number: 11888
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Comment-Date: Wed, 05 Dec 2018 21:42:38 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1531/ : 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/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 9 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Tue, 04 Dec 2018 19:25:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/11888 )
Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement
..
Patch Set 9:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11888/8/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/11888/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1140
PS8, Line 1140: if (sentryProxy_ != null) {
> Isn't this more of a precondition? (since we have an analyzer check for it)
Actually we set sentryProxy_ to null in
https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java#L274-L278.
In other words, we don't want to call this method when authorization is not
enabled via checking if sentryProxy_ != null.
http://gerrit.cloudera.org:8080/#/c/11888/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1146
PS8, Line 1146: Error refreshing authorization polic
> Update?
Done
http://gerrit.cloudera.org:8080/#/c/11888/8/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/11888/8/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3451
PS8, Line 3451: catalog_.refreshAuthorization(false);
> As discussed offline, I think we need to set updated/deleted objects here t
We actually increment the catalog version in L3452. I did a quick experiment by
increasing --statestore_update_frequency_ms to a higher value and the operation
blocks in proportion to the statestore update frequency value.
--
To view, visit http://gerrit.cloudera.org:8080/11888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c
Gerrit-Change-Number: 11888
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Comment-Date: Tue, 04 Dec 2018 18:54:56 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. IMPALA-7795: Implement REFRESH AUTHORIZATION statement This patch implements REFRESH AUTHORIZATION statement to explicitly refresh authorization metadata. This statement is useful to force Impala to refresh its authorization metadata when there is an external update to authorization metadata without having to wait for the Sentry polling or call INVALIDATE METADATA. Some tests were updated to use REFRESH AUTHORIZATION instead of INVALIDATE METADATA to make the tests run faster. Syntax: REFRESH AUTHORIZATION (authorization must be enabled to execute this statement) Testing: - Added new FE tests - Added new E2E authorization tests - Ran all FE tests - Ran all E2E authorization tests Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c --- M common/thrift/CatalogService.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.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/jflex/sql-scanner.flex M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M tests/authorization/test_authorization.py M tests/authorization/test_grant_revoke.py M tests/authorization/test_owner_privileges.py M tests/common/sentry_cache_test_suite.py 16 files changed, 263 insertions(+), 97 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/11888/9 -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 9 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/11888 )
Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement
..
Patch Set 8:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11888/8/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/11888/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1140
PS8, Line 1140: if (sentryProxy_ != null) {
Isn't this more of a precondition? (since we have an analyzer check for it)
http://gerrit.cloudera.org:8080/#/c/11888/8/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1146
PS8, Line 1146: Error updating authorization policy:
Update?
http://gerrit.cloudera.org:8080/#/c/11888/8/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/11888/8/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3451
PS8, Line 3451: catalog_.refreshAuthorization(false);
As discussed offline, I think we need to set updated/deleted objects here to
make it appear sync from a coordinator perspective. Can you double check?
--
To view, visit http://gerrit.cloudera.org:8080/11888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c
Gerrit-Change-Number: 11888
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Comment-Date: Mon, 03 Dec 2018 21:01:17 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1422/ : 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/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 8 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 21 Nov 2018 19:47:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 8: Code-Review+1 (1 comment) Carry Csaba's +1. http://gerrit.cloudera.org:8080/#/c/11888/7/tests/authorization/test_grant_revoke.py File tests/authorization/test_grant_revoke.py: http://gerrit.cloudera.org:8080/#/c/11888/7/tests/authorization/test_grant_revoke.py@121 PS7, Line 121: > nit: I would prefer to split this differently: Done -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 8 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 21 Nov 2018 19:06:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. IMPALA-7795: Implement REFRESH AUTHORIZATION statement This patch implements REFRESH AUTHORIZATION statement to explicitly refresh authorization metadata. This statement is useful to force Impala to refresh its authorization metadata when there is an external update to authorization metadata without having to wait for the Sentry polling or call INVALIDATE METADATA. Some tests were updated to use REFRESH AUTHORIZATION instead of INVALIDATE METADATA to make the tests run faster. Syntax: REFRESH AUTHORIZATION (authorization must be enabled to execute this statement) Testing: - Added new FE tests - Added new E2E authorization tests - Ran all FE tests - Ran all E2E authorization tests Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c --- M common/thrift/CatalogService.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.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/jflex/sql-scanner.flex M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M tests/authorization/test_authorization.py M tests/authorization/test_grant_revoke.py M tests/authorization/test_owner_privileges.py M tests/common/sentry_cache_test_suite.py 16 files changed, 263 insertions(+), 97 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/11888/8 -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 8 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 7: Code-Review+1 (1 comment) lgtm, but I am not confident around locks in the catalogd to give a +2 http://gerrit.cloudera.org:8080/#/c/11888/7/tests/authorization/test_grant_revoke.py File tests/authorization/test_grant_revoke.py: http://gerrit.cloudera.org:8080/#/c/11888/7/tests/authorization/test_grant_revoke.py@121 PS7, Line 121: TestObject nit: I would prefer to split this differently: self._execute_with_grant_option_tests( TestObject(TestObject.SERVER), priv, refresh) -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 21 Nov 2018 17:34:06 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1410/ : 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/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Tue, 20 Nov 2018 19:59:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. IMPALA-7795: Implement REFRESH AUTHORIZATION statement This patch implements REFRESH AUTHORIZATION statement to explicitly refresh authorization metadata. This statement is useful to force Impala to refresh its authorization metadata when there is an external update to authorization metadata without having to wait for the Sentry polling or call INVALIDATE METADATA. Some tests were updated to use REFRESH AUTHORIZATION instead of INVALIDATE METADATA to make the tests run faster. Syntax: REFRESH AUTHORIZATION (authorization must be enabled to execute this statement) Testing: - Added new FE tests - Added new E2E authorization tests - Ran all FE tests - Ran all E2E authorization tests Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c --- M common/thrift/CatalogService.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.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/jflex/sql-scanner.flex M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M tests/authorization/test_authorization.py M tests/authorization/test_grant_revoke.py M tests/authorization/test_owner_privileges.py M tests/common/sentry_cache_test_suite.py 16 files changed, 259 insertions(+), 90 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/11888/7 -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 7: (5 comments) http://gerrit.cloudera.org:8080/#/c/11888/6/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/11888/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3358 PS6, Line 3358:* 4) perform a refresh on authorization metadata. > 'refresh authorization' does not match any of these three items, so it coul Done http://gerrit.cloudera.org:8080/#/c/11888/6/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java File fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java: http://gerrit.cloudera.org:8080/#/c/11888/6/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java@1470 PS6, Line 1470: : /** :* Test admin functions are output correctly. :*/ > This is done in more than places. A function like createTestAuthConfig coul Done http://gerrit.cloudera.org:8080/#/c/11888/6/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java@1474 PS6, Line 1474: > createAnalysisCtx has an overload that expects a single AuthorizationConfig Done http://gerrit.cloudera.org:8080/#/c/11888/6/tests/authorization/test_authorization.py File tests/authorization/test_authorization.py: http://gerrit.cloudera.org:8080/#/c/11888/6/tests/authorization/test_authorization.py@541 PS6, Line 541: on > typo: one? Done http://gerrit.cloudera.org:8080/#/c/11888/6/tests/authorization/test_authorization.py@556 PS6, Line 556: s > 'unique_role' could also get a cleanup method similarly to 'unique_database Initially I thought of doing that, but creating a role requires Sentry to be enabled, the user needs to be part of Sentry admin, which can complicate things. -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Tue, 20 Nov 2018 19:32:06 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Csaba Ringhofer has posted comments on this change. (
http://gerrit.cloudera.org:8080/11888 )
Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement
..
Patch Set 6:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/11888/6/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/11888/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3358
PS6, Line 3358:*
'refresh authorization' does not match any of these three items, so it could be
added as fourth one.
http://gerrit.cloudera.org:8080/#/c/11888/6/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
File fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java:
http://gerrit.cloudera.org:8080/#/c/11888/6/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java@1470
PS6, Line 1470: AuthorizationConfig authzConfig =
AuthorizationConfig.createHadoopGroupAuthConfig(
: "server1", null, System.getenv("IMPALA_HOME") +
: "/fe/src/test/resources/sentry-site.xml");
: authzConfig.validateConfig();
This is done in more than places. A function like createTestAuthConfig could be
added to FrontendTestBase to avoid the duplication.
http://gerrit.cloudera.org:8080/#/c/11888/6/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java@1474
PS6, Line 1474: System.getProperty("user.name")
createAnalysisCtx has an overload that expects a single AuthorizationConfig and
uses user.name as username.
http://gerrit.cloudera.org:8080/#/c/11888/6/tests/authorization/test_authorization.py
File tests/authorization/test_authorization.py:
http://gerrit.cloudera.org:8080/#/c/11888/6/tests/authorization/test_authorization.py@541
PS6, Line 541: on
typo: one?
http://gerrit.cloudera.org:8080/#/c/11888/6/tests/authorization/test_authorization.py@556
PS6, Line 556: s
'unique_role' could also get a cleanup method similarly to 'unique_database' to
avoid this try/finally blocks.
--
To view, visit http://gerrit.cloudera.org:8080/11888
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c
Gerrit-Change-Number: 11888
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Comment-Date: Tue, 20 Nov 2018 17:56:59 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has removed Vuk Ercegovac from this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Removed reviewer Vuk Ercegovac. -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1349/ : 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/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 09 Nov 2018 20:17:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 6: Rebased. -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 09 Nov 2018 19:47:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. IMPALA-7795: Implement REFRESH AUTHORIZATION statement This patch implements REFRESH AUTHORIZATION statement to explicitly refresh authorization metadata. This statement is useful to force Impala to refresh its authorization metadata when there is an external update to authorization metadata without having to wait for the Sentry polling or call INVALIDATE METADATA. Some tests were updated to use REFRESH AUTHORIZATION instead of INVALIDATE METADATA to make the tests run faster. Syntax: REFRESH AUTHORIZATION (authorization must be enabled to execute this statement) Testing: - Added new FE tests - Added new E2E authorization tests - Ran all FE tests - Ran all E2E authorization tests Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c --- M common/thrift/CatalogService.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.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/jflex/sql-scanner.flex M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M tests/authorization/test_authorization.py M tests/authorization/test_grant_revoke.py M tests/authorization/test_owner_privileges.py M tests/common/sentry_cache_test_suite.py 15 files changed, 261 insertions(+), 86 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/11888/6 -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1316/ : 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/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 07 Nov 2018 21:36:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. IMPALA-7795: Implement REFRESH AUTHORIZATION statement This patch implements REFRESH AUTHORIZATION statement to explicitly refresh authorization metadata. This statement is useful to force Impala to refresh its authorization metadata when there is an external update to authorization metadata without having to wait for the Sentry polling or call INVALIDATE METADATA. Some tests were updated to use REFRESH AUTHORIZATION instead of INVALIDATE METADATA to make the tests run faster. Syntax: REFRESH AUTHORIZATION (authorization must be enabled to execute this statement) Testing: - Added new FE tests - Added new E2E authorization tests - Ran all FE tests - Ran all E2E authorization tests Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c --- M common/thrift/CatalogService.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.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/jflex/sql-scanner.flex M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M tests/authorization/test_authorization.py M tests/authorization/test_grant_revoke.py M tests/authorization/test_owner_privileges.py M tests/common/sentry_cache_test_suite.py 15 files changed, 261 insertions(+), 86 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/11888/5 -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1294/ : 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/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 06 Nov 2018 21:22:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/1293/ : 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/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 06 Nov 2018 21:12:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. IMPALA-7795: Implement REFRESH AUTHORIZATION statement This patch implements REFRESH AUTHORIZATION statement to explicitly refresh authorization metadata. This statement is useful to force Impala to refresh its authorization metadata when there is an external update to authorization metadata without having to wait for the Sentry polling or call INVALIDATE METADATA. Some tests were updated to use REFRESH AUTHORIZATION instead of INVALIDATE METADATA to make the tests run faster. Syntax: REFRESH AUTHORIZATION (authorization must be enabled to execute this statement) Testing: - Added new FE tests - Added new E2E authorization tests - Ran all FE tests - Ran all E2E authorization tests Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c --- M common/thrift/CatalogService.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.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/jflex/sql-scanner.flex M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M tests/authorization/test_authorization.py M tests/authorization/test_grant_revoke.py M tests/authorization/test_owner_privileges.py M tests/common/sentry_cache_test_suite.py 15 files changed, 260 insertions(+), 86 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/11888/4 -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/11888/3/tests/common/sentry_cache_test_suite.py File tests/common/sentry_cache_test_suite.py: http://gerrit.cloudera.org:8080/#/c/11888/3/tests/common/sentry_cache_test_suite.py@66 PS3, Line 66: > flake8: E501 line too long (92 > 90 characters) Done -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 06 Nov 2018 20:33:54 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11888 ) Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/11888/3/tests/common/sentry_cache_test_suite.py File tests/common/sentry_cache_test_suite.py: http://gerrit.cloudera.org:8080/#/c/11888/3/tests/common/sentry_cache_test_suite.py@66 PS3, Line 66: n flake8: E501 line too long (92 > 90 characters) -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 06 Nov 2018 20:31:41 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7795: Implement REFRESH AUTHORIZATION statement
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11888 Change subject: IMPALA-7795: Implement REFRESH AUTHORIZATION statement .. IMPALA-7795: Implement REFRESH AUTHORIZATION statement This patch implements REFRESH AUTHORIZATION statement to explicitly refresh authorization metadata. This statement is useful to force Impala to refresh its authorization metadata when there is an external update to authorization metadata without having to wait for the Sentry polling or call INVALIDATE METADATA. Some tests were updated to use REFRESH AUTHORIZATION instead of INVALIDATE METADATA to make the tests run faster. Syntax: REFRESH AUTHORIZATION (authorization must be enabled to execute this statement) Testing: - Added new FE tests - Added new E2E authorization tests - Ran all FE tests - Ran all E2E authorization tests Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c --- M common/thrift/CatalogService.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.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/jflex/sql-scanner.flex M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M tests/authorization/test_authorization.py M tests/authorization/test_grant_revoke.py M tests/authorization/test_owner_privileges.py M tests/common/sentry_cache_test_suite.py 15 files changed, 259 insertions(+), 86 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/11888/3 -- To view, visit http://gerrit.cloudera.org:8080/11888 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I5459e1c97b12dee307e0cf85b94a9f66fd9d9a8c Gerrit-Change-Number: 11888 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya
