----------------------------------------------------------- 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