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/29 16:24:40 UTC
Review Request 70341: Break up DDLTask - extract View related operations

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70341/
-----------------------------------------------------------

Review request for hive and Zoltan Haindrich.


Bugs: HIVE-21523
    https://issues.apache.org/jira/browse/HIVE-21523


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 #3: extract all the view related operations from the old DDLTask, and move them under the new package.


Diffs
-----

  ql/src/java/org/apache/hadoop/hive/ql/ddl/alter/AlterMaterializedViewDesc.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/alter/AlterMaterializedViewRewriteOperation.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/alter/package-info.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/ddl/table/CreateViewOperation.java PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java f4281bdd7b 
  ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java baf635633d 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java 77e181863e 
  ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java 0405ee8f02 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 1e79f325f8 
  ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 0b6ff524b1 
  ql/src/java/org/apache/hadoop/hive/ql/plan/AlterMaterializedViewDesc.java 865d1431d1 
  ql/src/java/org/apache/hadoop/hive/ql/plan/CreateViewDesc.java b693fdb845 
  ql/src/java/org/apache/hadoop/hive/ql/plan/DDLWork.java 2b653a5d21 
  ql/src/java/org/apache/hadoop/hive/ql/plan/ImportTableDesc.java 381c3b54a5 
  ql/src/java/org/apache/hadoop/hive/ql/plan/LoadFileDesc.java 3abdc4859f 
  ql/src/java/org/apache/hadoop/hive/ql/plan/PlanUtils.java b668e40594 
  ql/src/test/results/clientnegative/create_view_failure1.q.out 2b9a324c06 
  ql/src/test/results/clientnegative/create_view_failure4.q.out 300715d4cf 
  ql/src/test/results/clientnegative/masking_mv.q.out 6d5a46982e 
  ql/src/test/results/clientnegative/materialized_view_no_cbo_rewrite_2.q.out b04004b0d2 
  ql/src/test/results/clientnegative/materialized_view_no_supported_op_rewrite_2.q.out 0b72e3cb83 
  ql/src/test/results/clientpositive/beeline/materialized_view_create_rewrite.q.out 37cdb0af43 
  ql/src/test/results/clientpositive/create_view.q.out 081ac6f9d3 
  ql/src/test/results/clientpositive/create_view_translate.q.out 34ffb33bbf 
  ql/src/test/results/clientpositive/explain_ddl.q.out c52eda11d7 
  ql/src/test/results/clientpositive/llap/explainuser_1.q.out 1ea8fdcbb2 
  ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite.q.out 6bf9ee5e54 
  ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_3.q.out e0efe3cc1e 
  ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_4.q.out 6c3ba6cd96 
  ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_dummy.q.out 734ae685ef 
  ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_rebuild_dummy.q.out b55287deb0 
  ql/src/test/results/clientpositive/llap/materialized_view_create_rewrite_time_window.q.out cf6a6bace1 
  ql/src/test/results/clientpositive/llap/materialized_view_partitioned.q.out 19f014b57e 
  ql/src/test/results/clientpositive/llap/materialized_view_partitioned_3.q.out 0153822e7f 
  ql/src/test/results/clientpositive/llap/selectDistinctStar.q.out e0d42892f8 
  ql/src/test/results/clientpositive/llap/union_top_level.q.out 574ce2ba45 
  ql/src/test/results/clientpositive/llap/vector_windowing.q.out 8774c04347 
  ql/src/test/results/clientpositive/masking_mv.q.out afb0ecb672 
  ql/src/test/results/clientpositive/spark/spark_explainuser_1.q.out b8403f4e39 
  ql/src/test/results/clientpositive/spark/union_top_level.q.out 06f1bae0bd 
  ql/src/test/results/clientpositive/tez/explainanalyze_3.q.out eca2bf9e09 
  ql/src/test/results/clientpositive/tez/explainuser_3.q.out 3cb41ad1f7 


Diff: https://reviews.apache.org/r/70341/diff/1/


Testing
-------

Every q test is still running.


Thanks,

Miklos Gergely