----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70646/ -----------------------------------------------------------
Review request for hive and Zoltan Haindrich. Bugs: HIVE-21725 https://issues.apache.org/jira/browse/HIVE-21725 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 #9: extract all the column and constraint related operations from the old DDLTask, and move them under the new package each. Diffs ----- ql/src/java/org/apache/hadoop/hive/ql/ddl/table/AlterTableUtils.java 3c6d7eada9 ql/src/java/org/apache/hadoop/hive/ql/ddl/table/AlterTableWithConstraintsDesc.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/ddl/table/AlterTableWithWriteIdDesc.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/ddl/table/AlterTableWithWriteIdOperation.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/ddl/table/column/AlterTableAddColumnsDesc.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/ddl/table/column/AlterTableAddColumnsOperation.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/ddl/table/column/AlterTableChangeColumnDesc.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/ddl/table/column/AlterTableChangeColumnOperation.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/ddl/table/column/AlterTableReplaceColumnsDesc.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/ddl/table/column/AlterTableReplaceColumnsOperation.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/ddl/table/column/AlterTableUpdateColumnsDesc.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/ddl/table/column/AlterTableUpdateColumnsOperation.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/ddl/table/column/ShowColumnsOperation.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/ddl/table/column/package-info.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/ddl/table/constaint/AlterTableAddConstraintDesc.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/ddl/table/constaint/AlterTableAddConstraintOperation.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/ddl/table/constaint/AlterTableDropConstraintDesc.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/ddl/table/constaint/AlterTableDropConstraintOperation.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/ddl/table/constaint/Constraints.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/ddl/table/constaint/package-info.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/ddl/table/info/DescTableDesc.java cdd1777767 ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 0c531bed51 ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 99d7f21228 ql/src/java/org/apache/hadoop/hive/ql/parse/repl/load/message/AddForeignKeyHandler.java bba769244b ql/src/java/org/apache/hadoop/hive/ql/parse/repl/load/message/AddNotNullConstraintHandler.java 90d9008a31 ql/src/java/org/apache/hadoop/hive/ql/parse/repl/load/message/AddPrimaryKeyHandler.java e8966ad7c4 ql/src/java/org/apache/hadoop/hive/ql/parse/repl/load/message/AddUniqueConstraintHandler.java 81f1c5ab20 ql/src/java/org/apache/hadoop/hive/ql/parse/repl/load/message/DropConstraintHandler.java 5f9f879f6f ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableDesc.java 8603521041 ql/src/java/org/apache/hadoop/hive/ql/plan/DDLWork.java 6cd84bb8ab ql/src/java/org/apache/hadoop/hive/ql/plan/ShowColumnsDesc.java 7047f56275 ql/src/test/queries/clientpositive/allow_change_col_type_par.q aad63705f7 ql/src/test/queries/clientpositive/avro_alter_table_update_columns.q 279d05d2e3 ql/src/test/queries/clientpositive/check_constraint.q 202110259d ql/src/test/queries/clientpositive/rename_column.q 96daf9d658 ql/src/test/queries/clientpositive/show_columns.q aa45bae9d5 ql/src/test/results/clientnegative/allow_change_col_type_par_neg.q.out 3f91e85a3a ql/src/test/results/clientnegative/alter_partition_change_col_dup_col.q.out 542e85cb89 ql/src/test/results/clientnegative/alter_partition_change_col_nonexist.q.out bc5a6f980f ql/src/test/results/clientnegative/alter_table_constraint_duplicate_pk.q.out acf65f2ff6 ql/src/test/results/clientnegative/alter_table_constraint_invalid_fk_col1.q.out 1617609ce2 ql/src/test/results/clientnegative/alter_table_constraint_invalid_fk_col2.q.out 47166ac6c2 ql/src/test/results/clientnegative/alter_table_constraint_invalid_fk_tbl1.q.out 49bc928ac1 ql/src/test/results/clientnegative/alter_table_constraint_invalid_fk_tbl2.q.out f5ac4ac54a ql/src/test/results/clientnegative/alter_table_constraint_invalid_pk_col.q.out 71689f70ee ql/src/test/results/clientnegative/alter_table_constraint_invalid_pk_tbl.q.out 792134cbe6 ql/src/test/results/clientnegative/alter_table_constraint_invalid_ref.q.out 9e98454f5b ql/src/test/results/clientnegative/altern1.q.out ff3c670864 ql/src/test/results/clientnegative/avro_add_column_extschema.q.out a1b1b9ca6e ql/src/test/results/clientnegative/column_rename1.q.out 01549c2665 ql/src/test/results/clientnegative/column_rename2.q.out 41c2219d10 ql/src/test/results/clientnegative/column_rename4.q.out d5729f127a ql/src/test/results/clientnegative/disallow_incompatible_type_change_on1.q.out 8fe4c05872 ql/src/test/results/clientnegative/disallow_incompatible_type_change_on2.q.out 6291feb931 ql/src/test/results/clientnegative/drop_invalid_constraint1.q.out 2cb3996015 ql/src/test/results/clientnegative/drop_invalid_constraint2.q.out 04352b40d7 ql/src/test/results/clientnegative/drop_invalid_constraint3.q.out 03e4bd6097 ql/src/test/results/clientnegative/drop_invalid_constraint4.q.out 473dec7a4c ql/src/test/results/clientnegative/hms_using_serde_alter_table_update_columns.q.out 202acd7e31 ql/src/test/results/clientnegative/orc_reorder_columns1.q.out c581f4e312 ql/src/test/results/clientnegative/orc_reorder_columns1_acid.q.out 8f7255c973 ql/src/test/results/clientnegative/orc_reorder_columns2.q.out 54dcdecf1c ql/src/test/results/clientnegative/orc_reorder_columns2_acid.q.out 6cae15b81a ql/src/test/results/clientnegative/orc_replace_columns1.q.out 13f3f1448b ql/src/test/results/clientnegative/orc_replace_columns1_acid.q.out 46caec214f ql/src/test/results/clientnegative/orc_replace_columns2.q.out 2316bbbf35 ql/src/test/results/clientnegative/orc_replace_columns2_acid.q.out e01b7b9edb ql/src/test/results/clientnegative/orc_replace_columns3.q.out a7b3b72ced ql/src/test/results/clientnegative/orc_replace_columns3_acid.q.out b82ad57f6e ql/src/test/results/clientnegative/orc_type_promotion1.q.out f45283664a ql/src/test/results/clientnegative/orc_type_promotion1_acid.q.out 49800a96df ql/src/test/results/clientnegative/orc_type_promotion2.q.out 740ee1e850 ql/src/test/results/clientnegative/orc_type_promotion2_acid.q.out 28c789a20a ql/src/test/results/clientnegative/orc_type_promotion3.q.out 4f97e3118e ql/src/test/results/clientnegative/orc_type_promotion3_acid.q.out a214985f94 ql/src/test/results/clientnegative/parquet_alter_part_table_drop_columns.q.out d22d9c8763 ql/src/test/results/clientpositive/allow_change_col_type_par.q.out e9570370ea ql/src/test/results/clientpositive/avro_alter_table_update_columns.q.out 7f74f6c141 ql/src/test/results/clientpositive/input3.q.out 0ac95780cb ql/src/test/results/clientpositive/llap/check_constraint.q.out 297d8928f8 ql/src/test/results/clientpositive/rename_column.q.out 43abc7f72a ql/src/test/results/clientpositive/show_columns.q.out 80d69a7276 Diff: https://reviews.apache.org/r/70646/diff/1/ Testing ------- Added new unit tests + all the old unit tests are still running fine. Thanks, Miklos Gergely