saihemanth-cloudera commented on code in PR #5343:
URL: https://github.com/apache/hive/pull/5343#discussion_r1679954620
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -63,10 +63,19 @@ private List<HivePrivilegeObject> getInputHObjs() {
PreCreateTableEvent event = (PreCreateTableEvent) preEventContext;
Table table = event.getTable();
String uri = getSdLocation(table.getSd());
+ // if the table is storagehandler based table, need storageuri as hive
privilege objects
+ // storagehandler type + cluster + location
+ String storagehandler_uri =
table.getParameters().getOrDefault("storage_handler", null);
+ if (storagehandler_uri != null && storagehandler_uri.contains("iceberg")){
+ storagehandler_uri = "hiveicebergstoragehandler://"+
table.getParameters().getOrDefault("metadata_location", "");
Review Comment:
Storage_uri formation like this is not recommended. Please use this method
https://github.com/apache/hive/blob/master/iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java#L1065-L1093
to obtain URI. This is consistent with what we do in HS2:
https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/command/CommandAuthorizerV2.java#L208
For reference you could do something like this:
https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/AlterTableEvent.java#L108-L112
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/HiveMetaStoreAuthorizableEvent.java:
##########
@@ -64,6 +64,10 @@ protected HivePrivilegeObject getHivePrivilegeObject(Table
table) {
table.getOwner(), table.getOwnerType());
}
+ protected HivePrivilegeObject getHivePrivilegeObjectStorageHandlerUri(String
storagehandler_uri) {
+ return new
HivePrivilegeObject(HivePrivilegeObject.HivePrivilegeObjectType.STORAGEHANDLER_URI,
null, storagehandler_uri);
Review Comment:
Owner name, Owner type should be included in the privilege object
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/events/CreateTableEvent.java:
##########
@@ -78,6 +87,12 @@ private List<HivePrivilegeObject> getOutputHObjs() {
Table table = event.getTable();
Database database = event.getDatabase();
String uri = getSdLocation(table.getSd());
+ // if the table is storagehandler based table, need storageuri as hive
privilege objects
+ // storagehandler type + cluster + location
+ String storagehandler_uri =
table.getParameters().getOrDefault("storage_handler", null);
+ if (storagehandler_uri != null && storagehandler_uri.contains("iceberg")){
+ storagehandler_uri = "hiveicebergstoragehandler://"+
table.getParameters().getOrDefault("metadata_location", "");
+ }
Review Comment:
Implement the above suggestion here and remove the above HivePrivilegeObject
in getInputHObjs()
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
