-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70448/
-----------------------------------------------------------

Review request for hive and Zoltan Haindrich.


Bugs: HIVE-21593
    https://issues.apache.org/jira/browse/HIVE-21593


Repository: hive-git


Description
-------

DDLTask is a huge class, more than 5000 lines long. The related DDLWork is also 
a huge class, which has a field for each DDL operation it supports. The goal is 
to refactor these in order to have everything cut into more handleable classes 
under the package  org.apache.hadoop.hive.ql.exec.ddl:

have a separate class for each operation
have a package for each operation group (database ddl, table ddl, etc), so the 
amount of classes under a package is more manageable
make all the requests (DDLDesc subclasses) immutable
DDLTask should be agnostic to the actual operations
right now let's ignore the issue of having some operations handled by DDLTask 
which are not actual DDL operations (lock, unlock, desc...)
In the interim time when there are two DDLTask and DDLWork classes in the code 
base the new ones in the new package are called DDLTask2 and DDLWork2 thus 
avoiding the usage of fully qualified class names where both the old and the 
new classes are in use.

Step #5: extract all the privilege related operations from the old DDLTask, and 
move them under the new package.


Diffs
-----

  ql/src/java/org/apache/hadoop/hive/ql/ddl/database/AlterDatabaseDesc.java 
547b3515c0 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/database/ShowCreateDatabaseDesc.java 
29dc266ebf 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/database/ShowDatabasesDesc.java 
4814fd3e8c 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/function/DescFunctionDesc.java 
7f1aa0c90e 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/function/ShowFunctionsDesc.java 
2affa32786 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/function/ShowFunctionsOperation.java 
d76312d691 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/CreateRoleDesc.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/CreateRoleOperation.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/DropRoleDesc.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/DropRoleOperation.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/GrantOperation.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/GrantRoleDesc.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/GrantRoleOperation.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/RevokeOperation.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/RevokeRoleDesc.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/RevokeRoleOperation.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/RoleUtils.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/SetRoleDesc.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/SetRoleOperation.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowCurrentRoleDesc.java 
PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowCurrentRoleOperation.java
 PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowGrantOperation.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowPrincipalsDesc.java 
PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowPrincipalsOperation.java
 PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowRoleGrantDesc.java 
PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowRoleGrantOperation.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowRolesDesc.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/ShowRolesOperation.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/privilege/package-info.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/table/DescTableDesc.java 0cfffd2032 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/table/ShowCreateTableDesc.java 
8fa1ef16aa 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/table/ShowTablePropertiesDesc.java 
72caa58607 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/table/ShowTableStatusDesc.java 
8c312a0c5e 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/table/ShowTablesDesc.java 
584433b0a0 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 269cd852bf 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/repl/bootstrap/load/LoadDatabase.java
 c892b40224 
  ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
d187d197a0 
  
ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/AuthorizationParseUtils.java
 de5c90769a 
  
ql/src/java/org/apache/hadoop/hive/ql/parse/authorization/HiveAuthorizationTaskFactoryImpl.java
 18ed6fb418 
  
ql/src/java/org/apache/hadoop/hive/ql/parse/repl/load/message/AlterDatabaseHandler.java
 7162375cdf 
  
ql/src/java/org/apache/hadoop/hive/ql/parse/repl/load/message/CreateDatabaseHandler.java
 41b6db6e9d 
  ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableDesc.java d70353e358 
  ql/src/java/org/apache/hadoop/hive/ql/plan/DDLWork.java c3863e19a7 
  ql/src/java/org/apache/hadoop/hive/ql/plan/GrantDesc.java b5f9a69093 
  ql/src/java/org/apache/hadoop/hive/ql/plan/GrantRevokeRoleDDL.java 07529d9627 
  ql/src/java/org/apache/hadoop/hive/ql/plan/PrincipalDesc.java 1d82b1902c 
  ql/src/java/org/apache/hadoop/hive/ql/plan/PrivilegeDesc.java 1cb328a845 
  ql/src/java/org/apache/hadoop/hive/ql/plan/PrivilegeObjectDesc.java 
f18a51b998 
  ql/src/java/org/apache/hadoop/hive/ql/plan/RevokeDesc.java 0e0db1f22c 
  ql/src/java/org/apache/hadoop/hive/ql/plan/RoleDDLDesc.java afe7faf7fc 
  ql/src/java/org/apache/hadoop/hive/ql/plan/ShowGrantDesc.java 23d786f9f4 
  
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java
 f690422bfe 
  
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/DefaultHiveAuthorizationTranslator.java
 853dcf8a81 
  ql/src/java/org/apache/hadoop/hive/ql/security/authorization/Privilege.java 
1e9c639460 
  
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PrivilegeType.java 
7037f2c0ed 
  
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveAuthorizationTranslator.java
 29ce9ed299 
  
ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/AuthorizationTestUtil.java
 fed0d0116e 
  
ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/PrivilegesTestBase.java
 a15fd5d0c0 
  
ql/src/test/org/apache/hadoop/hive/ql/parse/authorization/TestHiveAuthorizationTaskFactory.java
 9a8c032623 
  
ql/src/test/results/clientnegative/authorization_cannot_create_default_role.q.out
 f328beb5af 
  ql/src/test/results/clientnegative/authorization_caseinsensitivity.q.out 
8bc747eac0 
  ql/src/test/results/clientnegative/authorization_create_role_no_admin.q.out 
981c8cd691 
  ql/src/test/results/clientnegative/authorization_drop_admin_role.q.out 
8383f52312 
  ql/src/test/results/clientnegative/authorization_drop_role_no_admin.q.out 
637167b9ba 
  ql/src/test/results/clientnegative/authorization_fail_1.q.out fc52cb338f 
  ql/src/test/results/clientnegative/authorization_fail_8.q.out e1ed1ad15c 
  ql/src/test/results/clientnegative/authorization_grant_group.q.out 712a5abbf0 
  ql/src/test/results/clientnegative/authorization_grant_table_allpriv.q.out 
d613fe1244 
  ql/src/test/results/clientnegative/authorization_grant_table_dup.q.out 
795dc83089 
  ql/src/test/results/clientnegative/authorization_grant_table_fail1.q.out 
4dbb9e3c23 
  
ql/src/test/results/clientnegative/authorization_grant_table_fail_nogrant.q.out 
0656ae548c 
  ql/src/test/results/clientnegative/authorization_invalid_priv_v2.q.out 
d390d90834 
  ql/src/test/results/clientnegative/authorization_priv_current_role_neg.q.out 
330a06c2e3 
  ql/src/test/results/clientnegative/authorization_public_create.q.out 
7defa82871 
  ql/src/test/results/clientnegative/authorization_public_drop.q.out 6aaa1ffd15 
  ql/src/test/results/clientnegative/authorization_revoke_table_fail1.q.out 
61fa52ac82 
  ql/src/test/results/clientnegative/authorization_revoke_table_fail2.q.out 
1b41d6189e 
  ql/src/test/results/clientnegative/authorization_role_case.q.out 4908e5dc26 
  ql/src/test/results/clientnegative/authorization_role_cycles1.q.out 
9303c7e69e 
  ql/src/test/results/clientnegative/authorization_role_cycles2.q.out 
df27beeafd 
  ql/src/test/results/clientnegative/authorization_role_grant.q.out cb79651f82 
  ql/src/test/results/clientnegative/authorization_role_grant2.q.out ade675252f 
  ql/src/test/results/clientnegative/authorization_role_grant_nosuchrole.q.out 
481842cdd5 
  ql/src/test/results/clientnegative/authorization_role_grant_otherrole.q.out 
144b78701b 
  ql/src/test/results/clientnegative/authorization_role_grant_otheruser.q.out 
a0c6845468 
  ql/src/test/results/clientnegative/authorization_set_role_neg1.q.out 
ee056be75d 
  ql/src/test/results/clientnegative/authorization_set_role_neg2.q.out 
539ce39dec 
  ql/src/test/results/clientnegative/authorization_show_grant_otherrole.q.out 
736e69335f 
  
ql/src/test/results/clientnegative/authorization_show_grant_otheruser_all.q.out 
9adbd09016 
  
ql/src/test/results/clientnegative/authorization_show_grant_otheruser_alltabs.q.out
 dea22644c8 
  
ql/src/test/results/clientnegative/authorization_show_grant_otheruser_wtab.q.out
 47d5c7ab68 
  
ql/src/test/results/clientnegative/authorization_show_role_principals_no_admin.q.out
 8be27b5643 
  ql/src/test/results/clientnegative/authorization_show_roles_no_admin.q.out 
adc2788961 
  ql/src/test/results/clientnegative/authorization_table_grant_nosuchrole.q.out 
6eef774759 
  ql/src/test/results/clientnegative/authorize_grant_public.q.out 6872b785d4 
  ql/src/test/results/clientnegative/authorize_revoke_public.q.out ede74871d2 
  ql/src/test/results/clientpositive/tez/explainanalyze_3.q.out 235f8c9d42 
  ql/src/test/results/clientpositive/tez/explainuser_3.q.out 40d1c32153 


Diff: https://reviews.apache.org/r/70448/diff/1/


Testing
-------

All the q tests are still running, some were slightly updated due to the new 
(cleaner) explain.


Thanks,

Miklos Gergely

Reply via email to