zabetak commented on code in PR #4304: URL: https://github.com/apache/hive/pull/4304#discussion_r1270599762
########## ql/src/test/queries/clientnegative/authorization_droptable_fail_10.q: ########## @@ -0,0 +1,11 @@ +set hive.security.authorization.manager=org.apache.hadoop.hive.ql.security.authorization.DefaultHiveAuthorizationProvider; +set hive.security.authorization.enabled=false; + +CREATE DATABASE auth_db_fail_4; +use auth_db_fail_4; +create temporary table drop_table_temp_4 (key int, value string) partitioned by (ds string); + +-- Drop temporary table from current database with IF EXISTS + +set hive.security.authorization.enabled=true; +DROP TABLE IF EXISTS drop_table_temp_4; Review Comment: nit: Some files miss the line ending. ########## ql/src/test/queries/clientpositive/authorization_drop_table.q: ########## @@ -1,25 +1,96 @@ set hive.security.authorization.manager=org.apache.hadoop.hive.ql.security.authorization.DefaultHiveAuthorizationProvider; +set hive.security.authorization.enabled=true; + +-- Drop table command for non-existing DB + +DROP TABLE auth_db.auth_permanent_table; + +-- Drop table command for non-existing DB + +DROP TABLE IF EXISTS auth_db.auth_permanent_table; + +-- Drop non-existing table with DB Drop Privileges + +set hive.security.authorization.enabled=false; + +CREATE DATABASE auth_db; +GRANT DROP ON DATABASE auth_db TO USER hive_test_user; + +set hive.security.authorization.enabled=true; + +DROP TABLE auth_db.auth_permanent_table; + +-- Drop non-existing table with IF EXISTS clause with DB Drop Privileges + +DROP TABLE IF EXISTS auth_db.auth_permanent_table; + +--- Create tables for test + set hive.security.authorization.enabled=false; -Create database auth_drop_table; +create table auth_db.drop_table_auth_1 (key int, value string) partitioned by (ds string); +create table auth_db.drop_table_auth_2 (key int, value string); +CREATE TEMPORARY TABLE auth_temp_table_1(key STRING, c1 INT, c2 STRING) STORED AS TEXTFILE; +CREATE TEMPORARY TABLE auth_temp_table_2(key STRING, c1 INT, c2 STRING) STORED AS TEXTFILE; + +GRANT All on table auth_db.drop_table_auth_1 to user hive_test_user; +GRANT All on table auth_db.drop_table_auth_2 to user hive_test_user; + +-- Drop existing regular table -use auth_drop_table; +set hive.security.authorization.enabled=true; +DROP TABLE auth_db.drop_table_auth_1; + +-- Drop existing regular table with IF EXISTS + +DROP TABLE IF EXISTS auth_db.drop_table_auth_2; + +-- Drop temporary table + +DROP TABLE auth_db.auth_temp_table_1; + +-- Drop temporary table with IF EXISTS + +DROP TABLE IF EXISTS auth_db.auth_temp_table_2; + + +-- Drop non-existing table from current database -create table drop_table_auth_1 (key int, value string) partitioned by (ds string); +set hive.security.authorization.enabled=false; -grant All on table drop_table_auth_1 to user hive_test_user; +CREATE DATABASE auth_db_1; +use auth_db_1; +GRANT DROP ON DATABASE auth_db_1 TO USER hive_test_user; -GRANT DROP ON DATABASE auth_drop_table TO USER hive_test_user; +create table drop_table_auth_3 (key int, value string) partitioned by (ds string); +create table drop_table_auth_4 (key int, value string); +CREATE TEMPORARY TABLE auth_temp_table_1(key STRING, c1 INT, c2 STRING) STORED AS TEXTFILE; +CREATE TEMPORARY TABLE auth_temp_table_2(key STRING, c1 INT, c2 STRING) STORED AS TEXTFILE; -show grant user hive_test_user on table drop_table_auth_1; -CREATE TEMPORARY TABLE drop_temp_table LIKE drop_table_auth_1; +GRANT All on table auth_db_1.drop_table_auth_4 to user hive_test_user; +GRANT All on table auth_db_1.drop_table_auth_3 to user hive_test_user; Review Comment: Why do we use `GRANT ALL ON TABLE`? Isn't the `GRANT DROP ON DATABASE` sufficient for dropping the tables? I assume that it has something to do with the fact that `drop_table_auth_3` is partitioned but in that case I would expect that `GRANT DROP ON TABLE` should be sufficient. Also it seems a bit strange that for temporary tables we do not use `GRANT ALL ON TABLE`. If this is only relevant for partitioning I would put the partitioning DDLs under a separate test file exclusively for this purpose. ########## ql/src/test/queries/clientnegative/authorization_droptable_fail_3.q: ########## @@ -0,0 +1,14 @@ + + Review Comment: nit: useless blank lines -- 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]
