Merge pull request #17 from sxzh93/change_type

added change type, changed logic in alter_executor, varchar now has default length.

Author: DeanChensj

src/catalog/catalog.cpp

@@ -804,10 +804,9 @@ std::shared_ptr<TableCatalogObject> Catalog::GetTableObject(
  * @param txn the transaction Context
  * @return TransactionContext ResultType(SUCCESS or FAILURE)
  */
-ResultType Catalog::AlterTable(
-    UNUSED_ATTRIBUTE oid_t database_oid, UNUSED_ATTRIBUTE oid_t table_oid,
-    UNUSED_ATTRIBUTE std::unique_ptr<catalog::Schema> &new_schema,
-    UNUSED_ATTRIBUTE concurrency::TransactionContext *txn) {
+ResultType Catalog::AlterTable(oid_t database_oid, oid_t table_oid,
+                               std::unique_ptr<catalog::Schema> &new_schema,
+                               concurrency::TransactionContext *txn) {
   LOG_TRACE("AlterTable in Catalog");
 
   if (txn == nullptr)
@@ -864,7 +863,6 @@ ResultType Catalog::AlterTable(
             old_index->GetMetadata()->GetIndexType(),
             old_index->GetMetadata()->GetIndexConstraintType(),
             new_schema.get(),
-            // catalog::Schema::CopySchema(old_index->GetKeySchema()),
             catalog::Schema::CopySchema(new_schema.get(), new_key_attrs),
             new_key_attrs, old_index->GetMetadata()->HasUniqueKeys());
 
@@ -923,14 +921,20 @@ ResultType Catalog::AlterTable(
                   new_schema->GetColumn(new_column_id).GetType());
             } else {
               // otherwise, copy value in old table
-              // TODO: Change type if necessary
               val = result_tile->GetValue(i, it->second);
-              if (new_schema->GetColumn(new_column_id).GetType() != old_schema->GetColumn(it->second).GetType()) {
-                //change the value's type
-                val = val.CastAs(new_schema->GetColumn(new_column_id).GetType());
+              if (new_schema->GetColumn(new_column_id).GetType() !=
+                  old_schema->GetColumn(it->second).GetType()) {
+                // change the value's type
+                LOG_TRACE(
+                    "CASTED: %s TO %s", val.GetInfo().c_str(),
+                    new_schema->GetColumn(new_column_id).GetInfo().c_str());
+                auto casted_val =
+                    val.CastAs(new_schema->GetColumn(new_column_id).GetType());
+                tuple->SetValue(new_column_id, casted_val, pool_.get());
+              } else {
+                tuple->SetValue(new_column_id, val, pool_.get());
               }
             }
-            tuple->SetValue(new_column_id, val, pool_.get());
           }
           // insert new tuple into new table
           planner::InsertPlan node(new_table, std::move(tuple));
@@ -952,13 +956,13 @@ ResultType Catalog::AlterTable(
             txn);
         column_offset++;
       }
+      // TODO: Add gc logic
+      // txn->RecordDrop(database_oid, old_table->GetOid(), INVALID_OID);
 
       // Final step of physical change should be moved to commit time
       database->ReplaceTableWithOid(table_oid, new_table);
 
-      // Record table drop
-      txn->RecordDrop(database_oid, table_oid, INVALID_OID);
-
+      LOG_TRACE("Alter table with oid %d succeed.", table_oid);
     } catch (CatalogException &e) {
       LOG_TRACE("Alter table failed.");
       return ResultType::FAILURE;

src/catalog/column.cpp

@@ -34,6 +34,7 @@ void Column::SetInlined() {
   switch (column_type) {
     case type::TypeId::VARCHAR:
     case type::TypeId::VARBINARY:
+      is_inlined = false;
       break;  // No change of inlined setting
 
     default:

src/executor/alter_executor.cpp

@@ -114,13 +114,13 @@ bool AlterExecutor::AlterTable(const peloton::planner::AlterPlan &node,
   }
   std::unique_ptr<catalog::Schema> temp_schema(
       catalog::Schema::CopySchema(old_schema, column_ids));
-
+  auto columns = temp_schema->GetColumns();
   // Step 2: change column type if exists
   for (auto change_pair : node.GetChangedTypeColumns()) {
     bool is_found = false;
     oid_t i = 0;
-    for (; i < temp_schema->GetColumnCount(); ++i) {
-      if (temp_schema->GetColumn(i).GetName() == change_pair.first) {
+    for (; i < columns.size(); ++i) {
+      if (columns[i].GetName() == change_pair.first) {
         is_found = true;
         break;
       }
@@ -131,7 +131,12 @@ bool AlterExecutor::AlterTable(const peloton::planner::AlterPlan &node,
       txn->SetResult(ResultType::FAILURE);
       return false;
     } else {
-      temp_schema->ChangeColumnType(i, change_pair.second);
+      columns[i].SetType(change_pair.second);
+      columns[i].SetInlined();
+      columns[i].SetLength(type::VarlenType::GetTypeSize(change_pair.second));
+
+      // TODO: decide VARCHAR's size when change type
+      // if (change_pair.second == type::TypeId::VARCHAR) {}
     }
   }
 
@@ -143,10 +148,8 @@ bool AlterExecutor::AlterTable(const peloton::planner::AlterPlan &node,
       add_columns.push_back(column);
     }
   }
-  std::unique_ptr<catalog::Schema> add_column_schema(
-      new catalog::Schema(add_columns));
   // Check if added column exists
-  for (auto new_column : add_column_schema->GetColumns()) {
+  for (auto new_column : add_columns) {
     for (auto old_column : old_schema->GetColumns()) {
       if (new_column.GetName() == old_column.GetName()) {
         LOG_TRACE("Add column failed: Column %s already exists",
@@ -156,10 +159,9 @@ bool AlterExecutor::AlterTable(const peloton::planner::AlterPlan &node,
       }
     }
   }
-
+  columns.insert(columns.end(), add_columns.begin(), add_columns.end());
   // Construct new schema
-  std::unique_ptr<catalog::Schema> new_schema(catalog::Schema::AppendSchema(
-      temp_schema.get(), add_column_schema.get()));
+  std::unique_ptr<catalog::Schema> new_schema(new catalog::Schema(columns));
 
   // Copy and replace table content to new schema in catalog
   ResultType result = catalog::Catalog::GetInstance()->AlterTable(

src/parser/postgresparser.cpp

@@ -1811,6 +1811,7 @@ parser::AlterTableStatement *PostgresParser::AlterTransform(Node *root) {
             std::unique_ptr<ColumnDefinition> col_def(
                 new ColumnDefinition(cmd->name, type_id));
             result->changed_type_columns->push_back(std::move(col_def));
+            break;
           }
           default: {
             throw NotImplementedException(StringUtil::Format(

src/storage/database.cpp

@@ -112,6 +112,7 @@ storage::DataTable *Database::ReplaceTableWithOid(
     const oid_t table_oid, storage::DataTable *new_table) {
   {
     std::lock_guard<std::mutex> lock(database_mutex);
+    codegen::QueryCache::Instance().Remove(table_oid);
 
     oid_t table_offset = 0;
     for (auto table : tables) {