Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11314 )
Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER ...................................................................... Patch Set 24: (3 comments) nits/clarifications, otherwise lgtm. http://gerrit.cloudera.org:8080/#/c/11314/24/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/11314/24/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1017 PS24, Line 1017: * the privilege, it will be removed on the next refresh. ALTER DATABASE SET OWNER might be worthwhile to state that while hms state will correctly reflect the owner and the catalogd catalog will reflect the hms state, since sentry is updated async by hms, sentry's state may not reflect the state in hms. when an out-of-date syntry updates the catalogd, it will use sentries incorrect state. consequently, the state in catalogd may not always match the state in hms. there will be some period that we could wait for sentry and hms to be in sync. perhaps we wait for their state to settle. if it doesn't, we roll-back the hms state? not sure if hms would give you the xact to effectively roll-back. I'm fine with not doing this now, but if its something that may be of interest, perhaps a todo is useful. http://gerrit.cloudera.org:8080/#/c/11314/24/fe/src/test/resources/sentry-site.xml.template File fe/src/test/resources/sentry-site.xml.template: http://gerrit.cloudera.org:8080/#/c/11314/24/fe/src/test/resources/sentry-site.xml.template@81 PS24, Line 81: <!-- These properties enable HMS follower. --> nit: remove the spacing http://gerrit.cloudera.org:8080/#/c/11314/23/tests/authorization/test_owner_privileges.py File tests/authorization/test_owner_privileges.py: http://gerrit.cloudera.org:8080/#/c/11314/23/tests/authorization/test_owner_privileges.py@156 PS23, Line 156: > Can't use that since I want to create the database after the cluster starts create table if not exists 'foo' ... ? -- To view, visit http://gerrit.cloudera.org:8080/11314 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0 Gerrit-Change-Number: 11314 Gerrit-PatchSet: 24 Gerrit-Owner: Adam Holley <[email protected]> Gerrit-Reviewer: Adam Holley <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Fri, 07 Sep 2018 06:18:18 +0000 Gerrit-HasComments: Yes
