vikramahuja1001 commented on code in PR #6610:
URL: https://github.com/apache/hive/pull/6610#discussion_r3598291076
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/AuthorizationMetaStoreFilterHook.java:
##########
@@ -56,43 +59,88 @@ public List<String> filterTableNames(String catName, String
dbName, List<String>
return getFilteredObjectNames(getFilteredObjects(listObjs));
}
+ /**
+ * Filters the given list of tables down to those the current user is
authorized to see.
+ *
+ * <p>The method delegates authorization decisions to {@link
HiveAuthorizer#filterListCmdObjects},
Review Comment:
Have reused TablePrivilegeLookup and refactored the code.
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/AuthorizationMetaStoreFilterHook.java:
##########
@@ -56,43 +59,88 @@ public List<String> filterTableNames(String catName, String
dbName, List<String>
return getFilteredObjectNames(getFilteredObjects(listObjs));
}
+ /**
+ * Filters the given list of tables down to those the current user is
authorized to see.
+ *
+ * <p>The method delegates authorization decisions to {@link
HiveAuthorizer#filterListCmdObjects},
+ * then maps the returned {@link HivePrivilegeObject}s back to the original
{@link Table} objects.
+ * The mapping is O(n) using pre-built HashMaps, replacing an earlier O(n²)
nested-loop
+ * implementation that was prohibitively slow for large table lists.
+ *
+ * <p>Catalog-name matching preserves the semantics of the original code
across three cases:
+ * <ol>
+ * <li><b>obj.catName == null</b> — the privilege object carries no
catalog constraint;
+ * the first table in the input list with the matching db+table name
is returned,
+ * regardless of that table's catName.</li>
+ * <li><b>obj.catName != null, table.catName == null</b> — a table that
was stored without
+ * a catalog name is treated as a wildcard and matches any privilege
object catName
+ * when no exact-catName table is found.</li>
+ * <li><b>obj.catName != null, table.catName != null</b> — both sides must
match exactly;
+ * if they differ the table is excluded.</li>
+ * </ol>
+ *
+ * @param tableList the full list of tables returned by the MetaStore before
authorization
+ * @return the sub-list of tables the current user is permitted to list
+ * @throws MetaException if the authorizer throws an exception
+ */
@Override
public List<Table> filterTables(List<Table> tableList) throws MetaException {
List<HivePrivilegeObject> listObjs = tablesToPrivilegeObjs(tableList);
- return getFilteredTableList(getFilteredObjects(listObjs),tableList);
- }
+ List<HivePrivilegeObject> filteredObjs = getFilteredObjects(listObjs);
- private List<Table> getFilteredTableList(List<HivePrivilegeObject>
hivePrivilegeObjects, List<Table> tableList) {
- List<Table> ret = new ArrayList<>();
- for(HivePrivilegeObject hivePrivilegeObject:hivePrivilegeObjects) {
- String catName = hivePrivilegeObject.getCatName();
- String dbName = hivePrivilegeObject.getDbname();
- String tblName = hivePrivilegeObject.getObjectName();
- Table table = getFilteredTable(catName, dbName, tblName, tableList);
- if (table != null) {
- ret.add(table);
+ // Map 1: catName + dbName + tblName → exact catName match (case 3)
+ Map<String, Table> fullKeyMap = new HashMap<>(tableList.size() * 2);
+
+ // Map 2: dbName + tblName → first table regardless of catName (case 1:
obj.catName == null)
+ Map<String, Table> anyMap = new HashMap<>(tableList.size() * 2);
+
+ // Map 3: dbName + tblName → first table where table.catName == null
(case 2)
+ Map<String, Table> nullCatMap = new HashMap<>();
+
+ for (Table table : tableList) {
+ String partKey = dbTableKey(table.getDbName(), table.getTableName());
+ String fullKey = catDbTableKey(table.getCatName(), table.getDbName(),
table.getTableName());
+
+ fullKeyMap.put(fullKey, table);
+ anyMap.putIfAbsent(partKey, table); // first match wins
+ if (table.getCatName() == null) {
+ nullCatMap.putIfAbsent(partKey, table); // only null-catName tables
}
}
- return ret;
- }
- private Table getFilteredTable(String catName, String dbName, String
tblName, List<Table> tableList) {
- Table ret = null;
- for (Table table: tableList) {
- // do not check catalog name if catName is null
- if (catName != null && table.getCatName() != null &&
!catName.equals(table.getCatName())) {
- continue;
+ List<Table> ret = new ArrayList<>(filteredObjs.size());
+ for (HivePrivilegeObject obj : filteredObjs) {
+ Table table;
+ String partKey = dbTableKey(obj.getDbname(), obj.getObjectName());
+
+ if (obj.getCatName() == null) {
Review Comment:
Have refactored the code.
--
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]
