jfsii commented on code in PR #4258:
URL: https://github.com/apache/hive/pull/4258#discussion_r1176613457
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreFilterHook.java:
##########
@@ -85,15 +85,13 @@ default List<String> filterCatalogs(List<String> catalogs)
throws MetaException
List<String> filterTableNames(String catName, String dbName, List<String>
tableList)
throws MetaException;
- // Previously this was handled by filterTableNames. But it can't be anymore
because we can no
- // longer depend on a 1-1 mapping between table name and entry in the list.
/**
* Filter a list of TableMeta objects.
* @param tableMetas list of TableMetas to filter
* @return filtered table metas
* @throws MetaException something went wrong
*/
- List<TableMeta> filterTableMetas(String catName,String
dbName,List<TableMeta> tableMetas) throws MetaException;
+ List<TableMeta> filterTableMetas(List<TableMeta> tableMetas) throws
MetaException;
Review Comment:
I'll make this change. I had hoped to just drop this interface method since
I didn't see any indication of other systems implementing MetaStoreFilterHooks
(I generally think less dead code laying around the better), but I guess it is
better to be safe here.
##########
ql/src/test/queries/clientpositive/authorization_privilege_objects.q:
##########
@@ -0,0 +1,20 @@
+--! qt:authorizer
+set
hive.security.authorization.manager=org.apache.hadoop.hive.ql.security.authorization.plugin.sqlstd.SQLStdHiveAuthorizerFactoryForTest;
+set test.hive.authz.sstd.validator.outputPrivObjs=true;
+set hive.test.authz.sstd.hs2.mode=true;
+set user.name=testuser;
+
+CREATE DATABASE test_db;
+CREATE TABLE test_privs(i int);
+set user.name=testuser2;
+CREATE TABLE test_privs2(s string, i int);
+set user.name=testuser;
+SHOW DATABASES;
+SHOW TABLES;
Review Comment:
I am unsure what you are trying to highlight.
The SHOW TABLEs might hit getTableMeta - however the purpose of this test
isn't to specifically test getTableMeta, it is to show the actual
HivePrivilegeObjects that end up getting passed to the authorization
implementation(s). I couldn't find any other test that verified the
HivePrivilegeObject being generated for various commands. I added a few other
query types - SELECTs/INSERTs for example to add some coverage for them, but
this test could be expanded to include many more statements (I just felt maybe
trying to cover them all is a bit out of scope for this PR).
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/minihms/AbstractMetaStoreService.java:
##########
@@ -99,7 +99,7 @@ public void start(Map<MetastoreConf.ConfVars, String>
metastoreOverlay,
* @return The client connected to this service
* @throws MetaException if any Exception occurs during client configuration
*/
- public IMetaStoreClient getClient() throws MetaException {
Review Comment:
It allows access to methods like setProcessorCapabilities in the children
tests. I did not see any indication the tests were specifically designed to
test the IMetaStoreClient, so I felt it was safe to expose HiveMetaStoreClient
to have better access to HMSClient methods.
--
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]
