Merge pull request #9 from sxzh93/refactor_rename_alter_parser

Refactor rename alter parser

Author: Dingshilun

src/binder/bind_node_visitor.cpp

@@ -190,9 +190,6 @@ void BindNodeVisitor::Visit(parser::TransactionStatement *) {}
 void BindNodeVisitor::Visit(parser::AlterTableStatement *node) {
   node->TryBindDatabaseName(default_database_name_);
 }
-void BindNodeVisitor::Visit(parser::RenameFuncStatement *node) {
-  node->TryBindDatabaseName(default_database_name_);
-}
 void BindNodeVisitor::Visit(parser::AnalyzeStatement *node) {
   node->TryBindDatabaseName(default_database_name_);
 }

src/catalog/catalog.cpp

@@ -812,10 +812,20 @@ ResultType Catalog::AddColumn(
 }
 
 ResultType Catalog::DropColumn(
-    UNUSED_ATTRIBUTE const std::string &database_name,
-    UNUSED_ATTRIBUTE const std::string &table_name,
-    UNUSED_ATTRIBUTE const std::vector<std::string> &columns,
-    UNUSED_ATTRIBUTE concurrency::TransactionContext *txn) {
+    const std::string &database_name,
+    const std::string &table_name,
+    const std::vector<std::string> &columns,
+    concurrency::TransactionContext *txn) {
+  try {
+    oid_t table_oid = Catalog::GetInstance()
+        ->GetTableObject(database_name, table_name, txn)
+        ->GetTableOid();
+    for (std::string name: columns) {
+      catalog::ColumnCatalog::GetInstance()->DeleteColumn(table_oid, name, txn);
+    }
+  } catch(CatalogException &e){
+    return ResultType::FAILURE;
+  }
   return ResultType::SUCCESS;
 }
 

src/executor/alter_executor.cpp

@@ -22,7 +22,8 @@ namespace executor {
 // Constructor for alter table executor
 AlterExecutor::AlterExecutor(const planner::AbstractPlan *node,
                              ExecutorContext *executor_context)
-    : AbstractExecutor(node, executor_context) {}
+    : AbstractExecutor(node, executor_context),
+      isAlter_(!reinterpret_cast<const planner::AlterPlan *>(node)->IsRename()) {}
 
 // Initialize executor
 // Nothing to initialize for now
@@ -36,24 +37,29 @@ bool AlterExecutor::DInit() {
 bool AlterExecutor::DExecute() {
   LOG_TRACE("Executing Drop...");
   bool result = false;
-  const planner::RenamePlan &node = GetPlanNode<planner::RenamePlan>();
-  auto current_txn = executor_context_->GetTransaction();
-  PlanNodeType plan_node_type = node.GetPlanNodeType();
-  if (plan_node_type == PlanNodeType::RENAME) {
+  if (!isAlter_) {
+    const planner::AlterPlan &node = GetPlanNode<planner::AlterPlan>();
+    auto current_txn = executor_context_->GetTransaction();
     result = RenameColumn(node, current_txn);
-  } else if (plan_node_type == PlanNodeType::ALTER) {
-    LOG_TRACE("Will perform alter table operations");
   } else {
-    throw NotImplementedException(
-        StringUtil::Format("Plan node type not supported, %s",
-                           PlanNodeTypeToString(plan_node_type).c_str()));
+    const planner::AlterPlan &node = GetPlanNode<planner::AlterPlan>();
+    auto current_txn = executor_context_->GetTransaction();
+    AlterType type = node.GetAlterTableType();
+    switch (type) {
+      case AlterType::DROP:
+        result = DropColumn(node, current_txn);
+        break;
+      default:
+        throw NotImplementedException(StringUtil::Format(
+            "Alter Type not supported, %s", AlterTypeToString(type).c_str()));
+    }
   }
 
   return result;
 }
 
 bool AlterExecutor::RenameColumn(
-    const peloton::planner::RenamePlan &node,
+    const peloton::planner::AlterPlan &node,
     peloton::concurrency::TransactionContext *txn) {
   auto database_name = node.GetDatabaseName();
   auto table_name = node.GetTableName();
@@ -69,7 +75,27 @@ bool AlterExecutor::RenameColumn(
   if (txn->GetResult() == ResultType::SUCCESS) {
     LOG_TRACE("Rename column succeeded!");
 
-    // Add on succeed logic if necessary
+    // TODO Add on succeed logic if necessary
+  } else {
+    LOG_TRACE("Result is: %s", ResultTypeToString(txn->GetResult()).c_str());
+  }
+  return false;
+}
+
+bool AlterExecutor::DropColumn(const peloton::planner::AlterPlan &node,
+                               peloton::concurrency::TransactionContext *txn) {
+  auto database_name = node.GetDatabaseName();
+  auto table_name = node.GetTableName();
+  auto drop_columns = node.GetDroppedColumns();
+
+  ResultType result = catalog::Catalog::GetInstance()->DropColumn(
+      database_name, table_name, drop_columns, txn);
+  txn->SetResult(result);
+
+  if (txn->GetResult() == ResultType::SUCCESS) {
+    LOG_TRACE("Drop column succeed!");
+
+    // TODO Add on succeed logic if necessary
   } else {
     LOG_TRACE("Result is: %s", ResultTypeToString(txn->GetResult()).c_str());
   }

src/executor/plan_executor.cpp

@@ -336,8 +336,9 @@ executor::AbstractExecutor *BuildExecutorTree(
       child_executor =
           new executor::PopulateIndexExecutor(plan, executor_context);
       break;
-    case PlanNodeType::RENAME:
-      child_executor = new executor::AlterExecutor(plan, executor_context);
+    case PlanNodeType::ALTER:
+      child_executor =
+          new executor::AlterExecutor(plan, executor_context);
       break;
     default:
       LOG_ERROR("Unsupported plan node type : %s",

src/include/binder/bind_node_visitor.h

@@ -68,7 +68,6 @@ class BindNodeVisitor : public SqlNodeVisitor {
   void Visit(parser::CopyStatement *) override;
   void Visit(parser::AnalyzeStatement *) override;
   void Visit(parser::AlterTableStatement *) override;
-  void Visit(parser::RenameFuncStatement *) override;
 
   void Visit(expression::CaseExpression *expr) override;
   void Visit(expression::SubqueryExpression *expr) override;

src/include/common/internal_types.h

@@ -646,6 +646,8 @@ std::ostream &operator<<(std::ostream &os, const DropType &type);
 enum class AlterType {
   INVALID = INVALID_TYPE_ID,  // invalid alter type
   RENAME = 1,                 // rename table, column, database...
+  ADD = 2,
+  DROP = 3
 };
 std::string AlterTypeToString(AlterType type);
 AlterType StringToAlterType(const std::string &str);

src/include/common/sql_node_visitor.h

@@ -21,7 +21,6 @@ class CreateFunctionStatement;
 class InsertStatement;
 class DeleteStatement;
 class DropStatement;
-class RenameFuncStatement;
 class AlterTableStatement;
 class ExplainStatement;
 class PrepareStatement;
@@ -84,7 +83,6 @@ class SqlNodeVisitor {
   virtual void Visit(parser::CopyStatement *) {}
   virtual void Visit(parser::AnalyzeStatement *) {}
   virtual void Visit(parser::AlterTableStatement *) {}
-  virtual void Visit(parser::RenameFuncStatement *) {}
   virtual void Visit(parser::ExplainStatement *){};
 
   virtual void Visit(expression::ComparisonExpression *expr);

src/include/executor/alter_executor.h

@@ -15,7 +15,6 @@
 #include "concurrency/transaction_context.h"
 #include "executor/abstract_executor.h"
 #include "planner/alter_plan.h"
-#include "planner/rename_plan.h"
 
 namespace peloton {
 
@@ -42,8 +41,14 @@ class AlterExecutor : public AbstractExecutor {
 
   bool DExecute();
 
-  bool RenameColumn(const planner::RenamePlan &node,
+  bool RenameColumn(const planner::AlterPlan &node,
                     concurrency::TransactionContext *txn);
+
+  bool DropColumn(const planner::AlterPlan &node,
+                  concurrency::TransactionContext *txn);
+
+ private:
+  bool isAlter_;
 };
 
 }  // executor

src/include/optimizer/query_node_visitor.h

@@ -29,7 +29,6 @@ class CopyStatement;
 class AnalyzeStatement;
 class VariableSetStatement;
 class JoinDefinition;
-class RenameFuncStatement;
 class AlterTableStatement;
 struct TableRef;
 

src/include/optimizer/query_to_operator_transformer.h

@@ -63,7 +63,6 @@ class QueryToOperatorTransformer : public SqlNodeVisitor {
   void Visit(parser::UpdateStatement *op) override;
   void Visit(parser::CopyStatement *op) override;
   void Visit(parser::AnalyzeStatement *op) override;
-  void Visit(parser::RenameFuncStatement *op) override;
   void Visit(parser::AlterTableStatement *op) override;
   void Visit(expression::ComparisonExpression *expr) override;
   void Visit(expression::OperatorExpression *expr) override;