----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70819/#review215787 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/AlterTableTypes.java Lines 30 (patched) <https://reviews.apache.org/r/70819/#comment302667> I think this should be AlterTableType and not with "s" ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableArchiveUtils.java Lines 100 (patched) <https://reviews.apache.org/r/70819/#comment302663> these methods seem to be not that closely related to archiving... looking at these methods: it seems like they are just changing exception types/argument order/etc... ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableCompactOperation.java Lines 104 (patched) <https://reviews.apache.org/r/70819/#comment302664> 5 minutes? seems like a lot ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableUnarchiveOperation.java Lines 266 (patched) <https://reviews.apache.org/r/70819/#comment302665> I think these TODO messages are pointless... catalog/tablename etc should be adressed more with a tableref or something "adding" another argument would not really make things better ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableUnarchiveOperation.java Lines 295 (patched) <https://reviews.apache.org/r/70819/#comment302666> I know it's hard to find places for these kind of methods....probably ArchiveUtils? ql/src/test/results/clientpositive/druid/druidmini_dynamic_partition.q.out Lines 491-496 (original), 489-492 (patched) <https://reviews.apache.org/r/70819/#comment302662> It might seem odd at first to see this "Unset properties" happening during an INSERT statement. Can't we add a different task to invalidate stats? (followup?) - Zoltan Haindrich On June 8, 2019, 3:09 p.m., Miklos Gergely wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70819/ > ----------------------------------------------------------- > > (Updated June 8, 2019, 3:09 p.m.) > > > Review request for hive and Zoltan Haindrich. > > > Bugs: HIVE-21830 > https://issues.apache.org/jira/browse/HIVE-21830 > > > 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 #10: extract the alter table operations that left from the old DDLTask, > and move them under the new packages. > > > Diffs > ----- > > accumulo-handler/src/test/results/positive/accumulo_queries.q.out > c5379c7348 > > accumulo-handler/src/test/results/positive/accumulo_single_sourced_multi_insert.q.out > 1a5dde0602 > > druid-handler/src/java/org/apache/hadoop/hive/druid/DruidStorageHandler.java > 254d0a39a6 > hbase-handler/src/test/results/negative/hbase_ddl.q.out 4646def667 > hbase-handler/src/test/results/positive/hbase_ddl.q.out 779ca4d16a > hbase-handler/src/test/results/positive/hbase_queries.q.out adf8864363 > > hbase-handler/src/test/results/positive/hbase_single_sourced_multi_insert.q.out > d474b4d065 > hbase-handler/src/test/results/positive/hbasestats.q.out 367b479556 > ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java 554df3c6bf > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/AbstractAlterTableDesc.java > 432779b3f4 > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/AbstractAlterTableOperation.java > baf98da37a > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/AbstractAlterTableWithConstraintsDesc.java > 9babf2a1a9 > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/AlterTableTypes.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/column/AlterTableAddColumnsDesc.java > e40ba1819d > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/column/AlterTableChangeColumnDesc.java > ce3b97eb68 > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/column/AlterTableReplaceColumnsDesc.java > 3975f6682a > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/column/AlterTableUpdateColumnsDesc.java > 18485c9a81 > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/constaint/AlterTableAddConstraintDesc.java > 2077c7d7e6 > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/misc/AlterTableRenameDesc.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/misc/AlterTableRenameOperation.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/misc/AlterTableSetOwnerDesc.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/misc/AlterTableSetOwnerOperation.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/misc/AlterTableSetPropertiesDesc.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/misc/AlterTableSetPropertiesOperation.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/misc/AlterTableTouchDesc.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/misc/AlterTableTouchOperation.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/misc/AlterTableUnsetPropertiesDesc.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/misc/AlterTableUnsetPropertiesOperation.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableArchiveDesc.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableArchiveOperation.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableArchiveUtils.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableClusteredByDesc.java > 8aab47b840 > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableCompactDesc.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableCompactOperation.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableConcatenateOperation.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableIntoBucketsDesc.java > 680f31096e > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableNotClusteredDesc.java > a335d0dbdc > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableNotSkewedDesc.java > af67964c73 > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableNotSortedDesc.java > 11e8bf37eb > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableSetFileFormatDesc.java > 89bbb17aec > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableSetLocationDesc.java > c918bb9870 > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableSetSerdeDesc.java > 861139d41b > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableSetSerdePropsDesc.java > 381b94f38a > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableSetSkewedLocationDesc.java > afe2b0817b > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableSkewedByDesc.java > 6a6f397ef7 > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableUnarchiveDesc.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/AlterTableUnarchiveOperation.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 88ea73f8d5 > ql/src/java/org/apache/hadoop/hive/ql/exec/ExportTask.java 078691cd06 > ql/src/java/org/apache/hadoop/hive/ql/exec/ReplCopyTask.java dcf569feb8 > ql/src/java/org/apache/hadoop/hive/ql/exec/Task.java 956c4ffabc > > ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ExternalTableCopyTaskBuilder.java > 6bc3cd0e0b > ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplDumpTask.java > 2f72e23526 > ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplLoadTask.java > 88e6327eab > > ql/src/java/org/apache/hadoop/hive/ql/exec/repl/incremental/IncrementalLoadTasksBuilder.java > 13de791fb3 > ql/src/java/org/apache/hadoop/hive/ql/exec/repl/util/ReplUtils.java > f9f13e1a4c > ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java 7f8f9a7631 > ql/src/java/org/apache/hadoop/hive/ql/parse/AcidExportSemanticAnalyzer.java > 76415cf7e2 > > ql/src/java/org/apache/hadoop/hive/ql/parse/AlterTablePartMergeFilesDesc.java > 215da93d15 > ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java > a58ac2ffe9 > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > 808c5c1350 > > ql/src/java/org/apache/hadoop/hive/ql/parse/repl/load/message/RenameTableHandler.java > 53d998200c > ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableDesc.java 9aa7e73fa7 > ql/src/java/org/apache/hadoop/hive/ql/plan/AlterTableSimpleDesc.java > 759a14f95c > ql/src/java/org/apache/hadoop/hive/ql/plan/DDLWork.java 07feae32e7 > ql/src/test/org/apache/hadoop/hive/ql/parse/TestQBCompact.java 61d74de443 > ql/src/test/queries/clientpositive/acid_vectorization_original_tez.q > 50fb3e9ad9 > ql/src/test/queries/clientpositive/archive_multi.q b372ea2f01 > ql/src/test/queries/clientpositive/dbtxnmgr_compact2.q 6e560dd446 > ql/src/test/queries/clientpositive/set_tblproperties.q PRE-CREATION > ql/src/test/queries/clientpositive/table_set_owner.q PRE-CREATION > ql/src/test/queries/clientpositive/touch.q 8711b7775c > ql/src/test/results/clientnegative/alter_external_acid.q.out 69bba3b8bd > ql/src/test/results/clientnegative/alter_non_native.q.out 110c9f01e2 > ql/src/test/results/clientnegative/alter_table_wrong_db.q.out 641e09f221 > ql/src/test/results/clientnegative/archive1.q.out b53085578b > ql/src/test/results/clientnegative/archive2.q.out 471df78e72 > ql/src/test/results/clientnegative/archive_multi1.q.out 3fed11ed68 > ql/src/test/results/clientnegative/archive_multi2.q.out 134a342e28 > ql/src/test/results/clientnegative/archive_multi3.q.out a4cd806450 > ql/src/test/results/clientnegative/archive_multi4.q.out 4221d2570e > ql/src/test/results/clientnegative/archive_multi5.q.out 315fbe4884 > ql/src/test/results/clientnegative/archive_multi6.q.out e73ddd1449 > ql/src/test/results/clientnegative/compact_non_acid_table.q.out eab9e19ae6 > ql/src/test/results/clientnegative/mm_convert.q.out 496f8b5f29 > ql/src/test/results/clientnegative/strict_managed_tables2.q.out 85753a494c > ql/src/test/results/clientnegative/strict_managed_tables3.q.out 7de8a80779 > ql/src/test/results/clientnegative/temp_table_rename.q.out 0956ababf3 > ql/src/test/results/clientnegative/touch1.q.out cc1e52ec4f > ql/src/test/results/clientpositive/alter_rename_table.q.out 32919ea11f > ql/src/test/results/clientpositive/archive_multi.q.out 5222c3398f > ql/src/test/results/clientpositive/dbtxnmgr_compact2.q.out b749846030 > ql/src/test/results/clientpositive/druid/druidmini_dynamic_partition.q.out > 7506ca8bea > ql/src/test/results/clientpositive/druid/druidmini_mv.q.out 483c9c16d5 > ql/src/test/results/clientpositive/encrypted/encryption_move_tbl.q.out > 23dbced668 > ql/src/test/results/clientpositive/input3.q.out c521ed6c78 > ql/src/test/results/clientpositive/llap/orc_merge10.q.out 22b4371fda > ql/src/test/results/clientpositive/llap/orc_merge5.q.out 5b24776b1f > ql/src/test/results/clientpositive/llap/orc_merge6.q.out 396834045b > ql/src/test/results/clientpositive/llap/orc_merge7.q.out 8472cfbd8a > ql/src/test/results/clientpositive/llap/orc_merge_incompat2.q.out > 319bac7020 > ql/src/test/results/clientpositive/orc_merge10.q.out b23438183d > ql/src/test/results/clientpositive/orc_merge5.q.out 88d93deae9 > ql/src/test/results/clientpositive/orc_merge6.q.out bd016202d4 > ql/src/test/results/clientpositive/orc_merge_incompat2.q.out e5930b2b58 > ql/src/test/results/clientpositive/set_tblproperties.q.out PRE-CREATION > ql/src/test/results/clientpositive/spark/orc_merge5.q.out fe5a71ef5f > ql/src/test/results/clientpositive/spark/orc_merge6.q.out ccf766dac7 > ql/src/test/results/clientpositive/spark/orc_merge7.q.out 192f8c46c8 > ql/src/test/results/clientpositive/spark/orc_merge_incompat2.q.out > 2330d9e272 > ql/src/test/results/clientpositive/table_set_owner.q.out PRE-CREATION > ql/src/test/results/clientpositive/table_storage.q.out 4311350afb > > ql/src/test/results/clientpositive/tez/acid_vectorization_original_tez.q.out > adde2486e5 > ql/src/test/results/clientpositive/tez/explainanalyze_3.q.out 1ad26b5f66 > ql/src/test/results/clientpositive/tez/explainuser_3.q.out c07c6a3572 > ql/src/test/results/clientpositive/touch.q.out c239715299 > > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaHook.java > e33f5e43ec > > > Diff: https://reviews.apache.org/r/70819/diff/1/ > > > Testing > ------- > > Added some new q tests + all the previous tests are passing. > > > Thanks, > > Miklos Gergely > >
