kgyrtkirk commented on a change in pull request #2583: URL: https://github.com/apache/hive/pull/2583#discussion_r766538342
########## File path: ql/src/java/org/apache/hadoop/hive/ql/security/authorization/command/CommandAuthorizerV2.java ########## @@ -195,32 +196,34 @@ private static void addHivePrivObject(Entity privObject, Map<String, List<String tableName2Cols.get(Table.getCompleteName(table.getDbName(), table.getTableName())); hivePrivObject = new HivePrivilegeObject(privObjType, table.getDbName(), table.getTableName(), null, columns, actionType, null, null, table.getOwner(), table.getOwnerType()); - if (table.getStorageHandler() != null) { - //TODO: add hive privilege object for storage based handlers for create and alter table commands. - if (hiveOpType == HiveOperationType.CREATETABLE || - hiveOpType == HiveOperationType.ALTERTABLE_PROPERTIES || - hiveOpType == HiveOperationType.CREATETABLE_AS_SELECT) { - String storageuri = null; - Map<String, String> tableProperties = new HashMap<>(); - Configuration conf = new Configuration(); - tableProperties.putAll(table.getSd().getSerdeInfo().getParameters()); - tableProperties.putAll(table.getParameters()); - try { - if (table.getStorageHandler() instanceof HiveStorageAuthorizationHandler) { - HiveStorageAuthorizationHandler authorizationHandler = (HiveStorageAuthorizationHandler) ReflectionUtils.newInstance( - conf.getClassByName(table.getStorageHandler().getClass().getName()), SessionState.get().getConf()); - storageuri = authorizationHandler.getURIForAuth(tableProperties).toString(); - } else { - //Custom storage handler that has not implemented the HiveStorageAuthorizationHandler - storageuri = table.getStorageHandler().getClass().getSimpleName().toLowerCase() + "://" + - HiveCustomStorageHandlerUtils.getTablePropsForCustomStorageHandler(tableProperties); + if (HiveConf.getBoolVar(SessionState.getSessionConf(), HiveConf.ConfVars.HIVE_AUTHORIZATION_TABLES_ON_STORAGEHANDLERS, true)) { + if (table.getStorageHandler() != null) { Review comment: could we join the 2 ifs instead? ```suggestion if (table.getStorageHandler() != null && HiveConf.getBoolVar(SessionState.getSessionConf(), HiveConf.ConfVars.HIVE_AUTHORIZATION_TABLES_ON_STORAGEHANDLERS, true)) { ``` -- 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: gitbox-unsubscr...@hive.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org