Adam Holley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11314 )
Change subject: IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER ...................................................................... Patch Set 20: (3 comments) carry +1 http://gerrit.cloudera.org:8080/#/c/11314/20/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/20/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@999 PS20, Line 999: // If object ownership is enabled in Sentry, we need to update the owner privilege : // in the catalog so that any subsequent statements that rely on that privilege, or : // the absence, will function correctly without waiting for the next refresh. : // e.g. For create, the privileges should be available to immediately create a table. : // Additionally, if the metadata operation is successful, but sentry fails to add : // the privilege, it will be removed on the next refresh. ALTER DATABASE SET OWNER : // can be used to try adding the owner privilege again. : if (isObjectOwnershipEnabled()) { : Preconditions.checkNotNull(params.server_name); : TPrivilege filter = createDatabaseOwnerPrivilege(newDb.getName(), : params.server_name); : addPrivilegeToCatalog(newDb.getMetaStoreDb().getOwnerName(), : newDb.getMetaStoreDb().getOwnerType(), filter, resp); : } > It would be nice to move similar blocks into separate functions. Done http://gerrit.cloudera.org:8080/#/c/11314/20/fe/src/test/resources/mysql-hive-site.xml.template File fe/src/test/resources/mysql-hive-site.xml.template: http://gerrit.cloudera.org:8080/#/c/11314/20/fe/src/test/resources/mysql-hive-site.xml.template@153 PS20, Line 153: <!-- These properties are for enabling the notifications between Hive and Sentry --> : <property> : <name>hive.metastore.transactional.event.listeners</name> : <value>org.apache.hive.hcatalog.listener.DbNotificationListener</value> : </property> : <property> : <name>hive.metastore.event.listeners</name> : <value>org.apache.sentry.binding.metastore.SentrySyncHMSNotificationsPostEventListener</value> : </property> : <property> : <name>hcatalog.message.factory.impl.json</name> : <value>org.apache.sentry.binding.metastore.messaging.json.SentryJSONMessageFactory</value> : </property> > How is this change related to the patch? Maybe it could be mentioned in the Done http://gerrit.cloudera.org:8080/#/c/11314/20/tests/authorization/test_owner_privileges.py File tests/authorization/test_owner_privileges.py: http://gerrit.cloudera.org:8080/#/c/11314/20/tests/authorization/test_owner_privileges.py@250 PS20, Line 250: __root_query_fail > As I see this functions are always used in pair with __verify_exceptions(). Done -- To view, visit http://gerrit.cloudera.org:8080/11314 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1e09332e007ed5aa6a0840683c879a8295c3d2b0 Gerrit-Change-Number: 11314 Gerrit-PatchSet: 20 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-Comment-Date: Wed, 05 Sep 2018 18:33:35 +0000 Gerrit-HasComments: Yes
