kgyrtkirk commented on a change in pull request #543: HIVE-21292: Break up 
DDLTask 1 - extract Database related operations
URL: https://github.com/apache/hive/pull/543#discussion_r259363654
 
 

 ##########
 File path: 
ql/src/java/org/apache/hadoop/hive/ql/exec/ddl/database/AlterDatabaseDesc.java
 ##########
 @@ -16,112 +16,100 @@
  * limitations under the License.
  */
 
-package org.apache.hadoop.hive.ql.plan;
+package org.apache.hadoop.hive.ql.exec.ddl.database;
 
 import java.io.Serializable;
 import java.util.Map;
 
+import org.apache.hadoop.hive.ql.exec.ddl.DDLDesc;
+import org.apache.hadoop.hive.ql.exec.ddl.DDLTask2;
 import org.apache.hadoop.hive.ql.parse.ReplicationSpec;
+import org.apache.hadoop.hive.ql.plan.Explain;
 import org.apache.hadoop.hive.ql.plan.Explain.Level;
+import org.apache.hadoop.hive.ql.plan.PrincipalDesc;
 
 /**
- * AlterDatabaseDesc.
- *
+ * DDL task description for ALTER DATABASE commands.
  */
 @Explain(displayName = "Alter Database", explainLevels = { Level.USER, 
Level.DEFAULT, Level.EXTENDED })
 public class AlterDatabaseDesc extends DDLDesc implements Serializable {
-
   private static final long serialVersionUID = 1L;
 
-  // Only altering the database property and owner is currently supported
-  public static enum ALTER_DB_TYPES {
-    ALTER_PROPERTY, ALTER_OWNER, ALTER_LOCATION
-  };
-
-  ALTER_DB_TYPES alterType;
-  String databaseName;
-  Map<String, String> dbProperties;
-  PrincipalDesc ownerPrincipal;
-  ReplicationSpec replicationSpec;
-  String location;
+  static {
+    DDLTask2.registerOperator(AlterDatabaseDesc.class, 
AlterDatabaseOperation.class);
+  }
 
   /**
-   * For serialization only.
+   * Supported type of alter db commands.
+   * Only altering the database property and owner is currently supported
    */
-  public AlterDatabaseDesc() {
-  }
+  public enum AlterDbType {
+    ALTER_PROPERTY, ALTER_OWNER, ALTER_LOCATION
+  };
 
-  public AlterDatabaseDesc(String databaseName, Map<String, String> dbProps,
 
 Review comment:
   I don't feel a few hundred lines much :) tought that closely related stuff 
might be beneficial during future feature additions/etc
   but it was just an idea; it will be fine without it as well.
   
   I'm right now wondering about what would happen if instead of adding an 
extra layer of  (`AlterDatabaseOperation`,`AlterDatabaseDesc`) ; how insane 
would be to do: `AlterDatabaseTask`,`Work` and `SA`?
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to