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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
