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

Reply via email to