You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2020/09/05 21:09:57 UTC
[GitHub] [hive] jcamachor commented on a change in pull request #1471: HIVE-23454 : Querying hive table which has Materialized view fails with HiveAccessControlException
jcamachor commented on a change in pull request #1471:
URL: https://github.com/apache/hive/pull/1471#discussion_r483989906
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
##########
@@ -1718,6 +1722,34 @@ public Table apply(org.apache.hadoop.hive.metastore.api.Table table) {
}
}
+ /**
+ * Validate if given materialized view has SELECT privileges for current user
+ * @param cachedMVTable
+ * @return false if user does not have privilege otherwise true
+ * @throws HiveException
+ */
+ private boolean checkPrivillegeForMV(final Table cachedMVTable) throws HiveException{
+ List<String> colNames =
+ cachedMVTable.getAllCols().stream()
+ .map(FieldSchema::getName)
+ .collect(Collectors.toList());
+
+ HivePrivilegeObject privObject = new HivePrivilegeObject(cachedMVTable.getDbName(),
+ cachedMVTable.getTableName(), colNames);
+ List<HivePrivilegeObject> privObjects = new ArrayList<HivePrivilegeObject>();
+ privObjects.add(privObject);
+ try {
+ SessionState.get().getAuthorizerV2().
+ checkPrivileges(HiveOperationType.QUERY, privObjects, privObjects, new HiveAuthzContext.Builder().build());
Review comment:
Can we check the privileges for all MVs used by the query at once so we do not need multiple round trips?
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
##########
@@ -1718,6 +1722,34 @@ public Table apply(org.apache.hadoop.hive.metastore.api.Table table) {
}
}
+ /**
+ * Validate if given materialized view has SELECT privileges for current user
+ * @param cachedMVTable
+ * @return false if user does not have privilege otherwise true
+ * @throws HiveException
+ */
+ private boolean checkPrivillegeForMV(final Table cachedMVTable) throws HiveException{
Review comment:
Can we move this to HiveMaterializedViewUtils?
There is also a typo: `Privillege`
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
##########
@@ -1736,6 +1768,10 @@ public boolean validateMaterializedViewsFromRegistry(List<Table> cachedMateriali
// Final result
boolean result = true;
for (Table cachedMaterializedViewTable : cachedMaterializedViewTables) {
+ if (!checkPrivillegeForMV(cachedMaterializedViewTable)) {
Review comment:
`validateMaterializedViewsFromRegistry` is only called if the MV is coming from the registry. However, we need the authorization check in all cases, e.g., dummy registry.
You can simply call the new method from Calcite planner.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org