You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hive.apache.org by Miklos Gergely <mg...@hortonworks.com> on 2019/03/11 13:54:00 UTC
Review Request 70176: Break up DDLTask - extract Table related
operations
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70176/
-----------------------------------------------------------
Review request for hive and Zoltan Haindrich.
Bugs: HIVE-21401
https://issues.apache.org/jira/browse/HIVE-21401
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 #2: extract all the table related operations from the old DDLTask except alter table, and move them under the new package. Also updated the framework a bit, and fixed some smaller issue related to the previous step.
Diffs
-----
accumulo-handler/src/test/results/positive/accumulo_queries.q.out ac2d527
accumulo-handler/src/test/results/positive/accumulo_single_sourced_multi_insert.q.out ac809fa
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java c33d03e
contrib/src/test/results/clientnegative/serde_regex.q.out 58a4679
contrib/src/test/results/clientpositive/fileformat_base64.q.out 8e6a5e4
contrib/src/test/results/clientpositive/serde_regex.q.out 691e254
hbase-handler/src/test/results/positive/hbase_ddl.q.out e87240a
hbase-handler/src/test/results/positive/hbase_queries.q.out 02f46d8
hbase-handler/src/test/results/positive/hbase_single_sourced_multi_insert.q.out b15515e
hbase-handler/src/test/results/positive/hbasestats.q.out 5143522
hcatalog/core/src/main/java/org/apache/hive/hcatalog/cli/SemanticAnalysis/CreateTableHook.java a377805
hcatalog/core/src/main/java/org/apache/hive/hcatalog/cli/SemanticAnalysis/HCatSemanticAnalyzer.java fd159fe
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 45aac5f
itests/util/src/main/java/org/apache/hadoop/hive/ql/metadata/DummySemanticAnalyzerHook.java 3575a16
itests/util/src/main/java/org/apache/hadoop/hive/ql/metadata/DummySemanticAnalyzerHook1.java e20ac64
ql/src/java/org/apache/hadoop/hive/ql/ddl/DDLOperation.java e349a0a
ql/src/java/org/apache/hadoop/hive/ql/ddl/DDLOperationContext.java 924f0b3
ql/src/java/org/apache/hadoop/hive/ql/ddl/DDLTask2.java 068e1e7
ql/src/java/org/apache/hadoop/hive/ql/ddl/DDLWork2.java d2fbe8f
ql/src/java/org/apache/hadoop/hive/ql/ddl/database/DescDatabaseOperation.java efaf389
ql/src/java/org/apache/hadoop/hive/ql/ddl/database/ShowCreateDatabaseDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/database/ShowCreateDatabaseOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/CreateTableDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/CreateTableLikeDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/CreateTableLikeOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/CreateTableOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/DescTableDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/DescTableOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/DropTableDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/DropTableOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/LockTableDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/LockTableOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/PreInsertTableDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/PreInsertTableOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/ShowCreateTableDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/ShowCreateTableOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/ShowTablePropertiesDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/ShowTablePropertiesOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/ShowTableStatusDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/ShowTableStatusOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/ShowTablesDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/ShowTablesOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/TruncateTableDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/TruncateTableOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/UnlockTableDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/UnlockTableOperation.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/package-info.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java a56695b
ql/src/java/org/apache/hadoop/hive/ql/exec/repl/bootstrap/load/table/LoadPartitions.java c1773c9
ql/src/java/org/apache/hadoop/hive/ql/exec/repl/bootstrap/load/table/LoadTable.java 3b0b67a
ql/src/java/org/apache/hadoop/hive/ql/io/AcidUtils.java 3961baa
ql/src/java/org/apache/hadoop/hive/ql/lockmgr/DbTxnManager.java 4aea872
ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManager.java 43dba73
ql/src/java/org/apache/hadoop/hive/ql/lockmgr/HiveTxnManagerImpl.java 0abec56
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 17576ff
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/JsonMetaDataFormatter.java a3ae886
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatUtils.java 4180dc4
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/MetaDataFormatter.java 80e3d8b
ql/src/java/org/apache/hadoop/hive/ql/metadata/formatting/TextMetaDataFormatter.java fbeb9c8
ql/src/java/org/apache/hadoop/hive/ql/optimizer/QueryPlanPostProcessor.java cf54aa3
ql/src/java/org/apache/hadoop/hive/ql/parse/AcidExportSemanticAnalyzer.java 4b2958a
ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 4a542ae
ql/src/java/org/apache/hadoop/hive/ql/parse/ImportSemanticAnalyzer.java b6b4f58
ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java 7b30b59
ql/src/java/org/apache/hadoop/hive/ql/parse/PreInsertTableDesc.java 2c8e1e1
ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java a2f6fbb
ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 05257c9
ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 8a51e21
ql/src/java/org/apache/hadoop/hive/ql/parse/repl/load/message/DropPartitionHandler.java b95a35a
ql/src/java/org/apache/hadoop/hive/ql/parse/repl/load/message/DropTableHandler.java 62784e9
ql/src/java/org/apache/hadoop/hive/ql/parse/repl/load/message/TruncatePartitionHandler.java dec6ed5
ql/src/java/org/apache/hadoop/hive/ql/parse/repl/load/message/TruncateTableHandler.java f037cbb
ql/src/java/org/apache/hadoop/hive/ql/plan/CreateTableDesc.java 4514af1
ql/src/java/org/apache/hadoop/hive/ql/plan/CreateTableLikeDesc.java 2cc0712
ql/src/java/org/apache/hadoop/hive/ql/plan/DDLWork.java 6527e52
ql/src/java/org/apache/hadoop/hive/ql/plan/DescTableDesc.java ee50232
ql/src/java/org/apache/hadoop/hive/ql/plan/DropPartitionDesc.java PRE-CREATION
ql/src/java/org/apache/hadoop/hive/ql/plan/DropTableDesc.java 5d22154
ql/src/java/org/apache/hadoop/hive/ql/plan/ImportTableDesc.java 017e1c7
ql/src/java/org/apache/hadoop/hive/ql/plan/LoadFileDesc.java 46761ff
ql/src/java/org/apache/hadoop/hive/ql/plan/LockTableDesc.java 723678e
ql/src/java/org/apache/hadoop/hive/ql/plan/PlanUtils.java 33a5371
ql/src/java/org/apache/hadoop/hive/ql/plan/ShowCreateDatabaseDesc.java ba5d06e
ql/src/java/org/apache/hadoop/hive/ql/plan/ShowCreateTableDesc.java f96c529
ql/src/java/org/apache/hadoop/hive/ql/plan/ShowTableStatusDesc.java 5022e28
ql/src/java/org/apache/hadoop/hive/ql/plan/ShowTablesDesc.java 0f7a3cd
ql/src/java/org/apache/hadoop/hive/ql/plan/ShowTblPropertiesDesc.java aac0cf2
ql/src/java/org/apache/hadoop/hive/ql/plan/TruncateTableDesc.java 61deb24
ql/src/java/org/apache/hadoop/hive/ql/plan/UnlockTableDesc.java 0b91463
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java 198f7fd
ql/src/test/org/apache/hadoop/hive/ql/parse/TestHiveDecimalParse.java 1ad0225
ql/src/test/queries/clientpositive/db_ddl_explain.q PRE-CREATION
ql/src/test/results/clientnegative/authorization_explain.q.out 792de42
ql/src/test/results/clientnegative/avro_decimal.q.out 9d00d6e
ql/src/test/results/clientnegative/constraint_duplicate_name.q.out 8a154f6
ql/src/test/results/clientnegative/create_external_acid.q.out 123fe5a
ql/src/test/results/clientnegative/create_not_acid.q.out e5aad61
ql/src/test/results/clientnegative/create_table_wrong_regex.q.out 931f2a7
ql/src/test/results/clientnegative/create_view_failure2.q.out ad5d5fe
ql/src/test/results/clientnegative/create_with_constraints_duplicate_name.q.out b3d1d9f
ql/src/test/results/clientnegative/create_with_fk_constraint.q.out 6598d6c
ql/src/test/results/clientnegative/create_with_fk_pk_same_tab.q.out fae2769
ql/src/test/results/clientnegative/create_with_fk_uk_same_tab.q.out 1644d5a
ql/src/test/results/clientnegative/create_with_fk_wrong_ref.q.out ce0f947
ql/src/test/results/clientnegative/create_with_fk_wrong_ref2.q.out 998c643
ql/src/test/results/clientnegative/dbtxnmgr_notablelock.q.out 3fad08c
ql/src/test/results/clientnegative/dbtxnmgr_notableunlock.q.out 2d9a20f
ql/src/test/results/clientnegative/deletejar.q.out d52186b
ql/src/test/results/clientnegative/describe_xpath1.q.out 322e6e8
ql/src/test/results/clientnegative/describe_xpath2.q.out c1f2ec1
ql/src/test/results/clientnegative/describe_xpath3.q.out a300633
ql/src/test/results/clientnegative/describe_xpath4.q.out b569eca
ql/src/test/results/clientnegative/drop_table_failure2.q.out f0097cd
ql/src/test/results/clientnegative/drop_table_used_by_mv.q.out 88e3b7d
ql/src/test/results/clientnegative/drop_view_failure1.q.out a1a4498
ql/src/test/results/clientnegative/druid_address.q.out 66b7e14
ql/src/test/results/clientnegative/druid_buckets.q.out 94e4f70
ql/src/test/results/clientnegative/druid_case.q.out 457028b
ql/src/test/results/clientnegative/druid_datasource.q.out 177ffaa
ql/src/test/results/clientnegative/druid_datasource2.q.out 2f783fe
ql/src/test/results/clientnegative/druid_location.q.out 5727e8c
ql/src/test/results/clientnegative/druid_partitions.q.out 6fb55c1
ql/src/test/results/clientnegative/external1.q.out f2bc9c6
ql/src/test/results/clientnegative/insert_sorted.q.out bb3c7e3
ql/src/test/results/clientnegative/lockneg1.q.out 3a96cda
ql/src/test/results/clientnegative/lockneg2.q.out 31e9087
ql/src/test/results/clientnegative/lockneg3.q.out e4f6357
ql/src/test/results/clientnegative/materialized_view_drop.q.out da95afb
ql/src/test/results/clientnegative/materialized_view_drop2.q.out d4f243c
ql/src/test/results/clientnegative/nested_complex_neg.q.out a6f9ac5
ql/src/test/results/clientnegative/serde_regex.q.out 1047a82
ql/src/test/results/clientnegative/serde_regex3.q.out 33d647b
ql/src/test/results/clientnegative/special_character_in_tabnames_1.q.out d7b9965
ql/src/test/results/clientnegative/strict_managed_tables1.q.out a659644
ql/src/test/results/clientnegative/strict_managed_tables4.q.out 0c7576f
ql/src/test/results/clientnegative/strict_managed_tables5.q.out 0e29fbd
ql/src/test/results/clientpositive/ambiguitycheck.q.out aff5752
ql/src/test/results/clientpositive/annotate_stats_table.q.out d7f7b22
ql/src/test/results/clientpositive/create_union_table.q.out f773f34
ql/src/test/results/clientpositive/ctas.q.out 7c7378a
ql/src/test/results/clientpositive/ctas_colname.q.out 9db4ddd
ql/src/test/results/clientpositive/ctas_uses_database_location.q.out eb3872e
ql/src/test/results/clientpositive/db_ddl_explain.q.out PRE-CREATION
ql/src/test/results/clientpositive/drop_deleted_partitions.q.out 85f4f53
ql/src/test/results/clientpositive/drop_multi_partitions.q.out 6b59749
ql/src/test/results/clientpositive/druid/druidmini_dynamic_partition.q.out d25b350
ql/src/test/results/clientpositive/druid/druidmini_mv.q.out 96690af
ql/src/test/results/clientpositive/explain_ddl.q.out a71925f
ql/src/test/results/clientpositive/fileformat_sequencefile.q.out 8610fa0
ql/src/test/results/clientpositive/fileformat_text.q.out 387f807
ql/src/test/results/clientpositive/groupby_duplicate_key.q.out 432ff08
ql/src/test/results/clientpositive/input1.q.out 63f8af0
ql/src/test/results/clientpositive/input10.q.out bbdff6e
ql/src/test/results/clientpositive/input15.q.out 2dbf6fb
ql/src/test/results/clientpositive/input2.q.out aada917
ql/src/test/results/clientpositive/inputddl1.q.out a95e9f1
ql/src/test/results/clientpositive/inputddl2.q.out a5ec1c9
ql/src/test/results/clientpositive/inputddl3.q.out 639f095
ql/src/test/results/clientpositive/inputddl6.q.out e14807c
ql/src/test/results/clientpositive/llap/ctas.q.out c761b9d
ql/src/test/results/clientpositive/llap/dynamic_partition_pruning.q.out adf8011
ql/src/test/results/clientpositive/llap/explainuser_1.q.out 1ee459b
ql/src/test/results/clientpositive/llap/partition_ctas.q.out 3e290b3
ql/src/test/results/clientpositive/llap/rcfile_createas1.q.out 4f1a479
ql/src/test/results/clientpositive/llap/semijoin_reddedup.q.out 1fca347
ql/src/test/results/clientpositive/llap/temp_table.q.out 45be750
ql/src/test/results/clientpositive/llap/tez_dml.q.out 1e8ab44
ql/src/test/results/clientpositive/llap/tez_join_result_complex.q.out c54a89b
ql/src/test/results/clientpositive/llap/unionDistinct_1.q.out 7f2cc85
ql/src/test/results/clientpositive/llap/union_top_level.q.out 8fc40fc
ql/src/test/results/clientpositive/llap/vector_char_varchar_1.q.out e919a70
ql/src/test/results/clientpositive/llap/vector_decimal_6.q.out ad1757c
ql/src/test/results/clientpositive/llap/vector_windowing_streaming.q.out 41d12ba
ql/src/test/results/clientpositive/llap/vectorized_dynamic_partition_pruning.q.out 21d4e1b
ql/src/test/results/clientpositive/merge3.q.out 00280e7
ql/src/test/results/clientpositive/nonReservedKeyWords.q.out 7d27c34
ql/src/test/results/clientpositive/nonmr_fetch.q.out 6d439af
ql/src/test/results/clientpositive/nullformat.q.out d14c570
ql/src/test/results/clientpositive/nullformatCTAS.q.out 1d9edbc
ql/src/test/results/clientpositive/orc_createas1.q.out aa6ba1b
ql/src/test/results/clientpositive/parallel_orderby.q.out 158e6ee
ql/src/test/results/clientpositive/serde_opencsv.q.out 1f80eeb
ql/src/test/results/clientpositive/serde_regex.q.out 1d00a49
ql/src/test/results/clientpositive/show_tables.q.out 18d7c59
ql/src/test/results/clientpositive/show_tablestatus.q.out 3fbc93f
ql/src/test/results/clientpositive/skewjoin_noskew.q.out f22cee0
ql/src/test/results/clientpositive/skewjoin_onesideskew.q.out 28b7c8b
ql/src/test/results/clientpositive/smb_mapjoin9.q.out cc78847
ql/src/test/results/clientpositive/spark/ctas.q.out 907ace2
ql/src/test/results/clientpositive/spark/parallel_orderby.q.out edb7793
ql/src/test/results/clientpositive/spark/skewjoin_noskew.q.out 7afa720
ql/src/test/results/clientpositive/spark/spark_dynamic_partition_pruning.q.out d3c55a3
ql/src/test/results/clientpositive/spark/spark_explainuser_1.q.out 0422dc2
ql/src/test/results/clientpositive/spark/spark_vectorized_dynamic_partition_pruning.q.out 6fbab46
ql/src/test/results/clientpositive/spark/temp_table.q.out 8ec449b
ql/src/test/results/clientpositive/spark/union25.q.out 6c80376
ql/src/test/results/clientpositive/spark/union_top_level.q.out 51fae27
ql/src/test/results/clientpositive/symlink_text_input_format.q.out fd8800c
ql/src/test/results/clientpositive/temp_table_truncate.q.out ba7133b
ql/src/test/results/clientpositive/tez/explainanalyze_1.q.out 77395ad
ql/src/test/results/clientpositive/tez/explainanalyze_3.q.out 4d34541
ql/src/test/results/clientpositive/tez/explainuser_3.q.out 2b2027c
ql/src/test/results/clientpositive/truncate_table.q.out 6ce5f3a
ql/src/test/results/clientpositive/union25.q.out 2423f5c
ql/src/test/results/clientpositive/vector_decimal_6.q.out f351f3e
ql/src/test/results/clientpositive/vector_tablesample_rows.q.out ff09824
Diff: https://reviews.apache.org/r/70176/diff/1/
Testing
-------
All the unit and q tests are still running.
Thanks,
Miklos Gergely