jfsii commented on code in PR #4601:
URL: https://github.com/apache/hive/pull/4601#discussion_r1293657388
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObjectUtils.java:
##########
@@ -59,7 +59,74 @@ public static List<HivePrivilegeObject>
getHivePrivDcObjects(List<String> dcList
objs.add(new HivePrivilegeObject(HivePrivilegeObjectType.DATACONNECTOR,
null, dcname));
}
return objs;
+ }
+
+
+ /**
+ * An index on a table list by dbName, tableName.
+ */
+ public abstract static class TableIndex<T> {
+ private final T[] index;
+ protected abstract String getDbName(T item);
+ protected abstract String getTableName(T item);
+ protected TableIndex(List<T> tables) {
+ if (tables != null) {
+ index = tables.toArray((T[]) new Object[0]);
+ // sort them by dbname, tblname
+ Arrays.sort(index, (left, right)->{
+ int cmp = getDbName(left).compareTo(getDbName(right));
+ return cmp != 0
+ ? cmp
+ : getTableName(left).compareTo(getTableName(right));
+ });
+ } else {
+ index = (T[]) new Object[0];
+ }
+ }
+ /**
+ * Lookup using dichotomy using order described by Name, tblName
+ * @param dbName the database name
+ * @param tableName the table name
+ * @return the table if found in the index, null otherwise
+ */
+ public T lookup(String dbName, String tableName) {
+ int low = 0;
+ int high = index.length - 1;
+ while (low <= high) {
+ int mid = (low + high) >>> 1;
+ T item = index[mid];
+ int cmp = getDbName(item).compareTo(dbName);
+ if (cmp == 0) {
+ cmp = getTableName(item).compareTo(tableName);
+ }
+ if (cmp < 0)
Review Comment:
I do not know the exact Hive code style here - but I usually prefer always
using { }'s unless it is single line.
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObjectUtils.java:
##########
@@ -59,7 +59,74 @@ public static List<HivePrivilegeObject>
getHivePrivDcObjects(List<String> dcList
objs.add(new HivePrivilegeObject(HivePrivilegeObjectType.DATACONNECTOR,
null, dcname));
}
return objs;
+ }
+
+
+ /**
+ * An index on a table list by dbName, tableName.
+ */
+ public abstract static class TableIndex<T> {
+ private final T[] index;
+ protected abstract String getDbName(T item);
+ protected abstract String getTableName(T item);
+ protected TableIndex(List<T> tables) {
+ if (tables != null) {
+ index = tables.toArray((T[]) new Object[0]);
+ // sort them by dbname, tblname
+ Arrays.sort(index, (left, right)->{
+ int cmp = getDbName(left).compareTo(getDbName(right));
Review Comment:
It might be a little more easy to read if there was a helper function that
had a signature like:
int compare(T item, String dbName, String tableName) or
int compare(String compareDbName, String compareTableName, String
compareToDbName, String compareToTableName)
which does the composite key compare and returns the cmp value and use it in
TableIndex constructor and lookup method.
The argument names were hard, if you take this suggestion, I'm sure you
could come up with better.
##########
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/HiveMetaStoreAuthorizer.java:
##########
@@ -396,32 +383,16 @@ private List<String>
filterTableNames(HiveMetaStoreAuthzInfo hiveMetaStoreAuthzI
}
return ret;
}
-
- private List<String> getFilteredTableNames(List<HivePrivilegeObject>
hivePrivilegeObjects, String databaseName,
- List<String> tableNames) {
+ private List<String> getFilteredTableNames(List<HivePrivilegeObject>
hivePrivilegeObjects, String databaseName, List<String> tableNames) {
List<String> ret = new ArrayList<>();
- for (HivePrivilegeObject hivePrivilegeObject : hivePrivilegeObjects) {
- String dbName = hivePrivilegeObject.getDbname();
- String tblName = hivePrivilegeObject.getObjectName();
- String table = getFilteredTableNames(dbName, tblName, databaseName,
tableNames);
- if (table != null) {
- ret.add(table);
- }
- }
- return ret;
- }
-
- private String getFilteredTableNames(String dbName, String tblName, String
databaseName, List<String> tableNames) {
- String ret = null;
- for (String tableName : tableNames) {
- if (dbName.equals(databaseName) && tblName.equals(tableName)) {
- ret = tableName;
- break;
+ final PrivilegeIndex index = new PrivilegeIndex(hivePrivilegeObjects);
+ for(String tableName : tableNames) {
+ if (index.lookup(databaseName, tableName) != null) {
+ ret.add(tableName);
}
}
return ret;
}
-
Review Comment:
I think this blank line was accidentally removed.
##########
ql/src/test/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/TestHiveMetaStoreAuthorizer.java:
##########
@@ -411,4 +418,93 @@ public void testU_DropDataConnector_authorizedUser() {
fail("testU_DropDataConnector_authorizedUser() failed with " + e);
}
}
+
+ /**
+ * Fake tables to test.
+ */
+ private static class TestTable {
+ private final String dbName;
+ private final String tableName;
+
+ private TestTable(String dbName, String tableName) {
+ this.dbName = dbName;
+ this.tableName = tableName;
+ }
+
+ @Override
+ public String toString() {
+ return dbName + "." + tableName;
+ }
+
+ public String getDbName() {
+ return dbName;
+ }
+
+ public String getTableName() {
+ return tableName;
+ }
+ }
+
+
+ private static List<TestTable> createTables(int dbs, int tbls) {
+ List<TestTable> tables = new ArrayList<>(dbs * tbls);
+ for(int d = 0; d < dbs; ++d) {
+ for(int t = 0; t < tbls; ++t) {
+ String dbname = String.format("db%05x", d);
+ String tblName = String.format("tbl%05x", t);
+ tables.add(new TestTable(dbname, tblName));
+ }
+ }
+ return tables;
+ }
+
+ private static final int NUM_DB = 2;
+ private static final int NUM_TBL = 50;
+ @Ignore
+ public void testIndexedTableLookup0() {
Review Comment:
Not sure of the purpose of this test, could you clarify or remove?
--
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
