fix bug during creating column: use a wrong db name (#107)

pingcap · Jul 16, 2019 · 9314f91 · 9314f91
1 parent 745850e
commit 9314f91
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 14 deletions.
diff --git a/dbms/src/Storages/StorageMergeTree.cpp b/dbms/src/Storages/StorageMergeTree.cpp
@@ -385,9 +385,9 @@ void StorageMergeTree::alter(
 void StorageMergeTree::alterForTMT(
     const AlterCommands & params,
     const TiDB::TableInfo & table_info,
+    const String & database_name,
     const Context & context)
 {
-    const String & database_name = table_info.db_name;
     const String & table_name = table_info.name;
     /// NOTE: Here, as in ReplicatedMergeTree, you can do ALTER which does not block the writing of data for a long time.
     auto merge_blocker = merger.merges_blocker.cancel();

diff --git a/dbms/src/Storages/StorageMergeTree.h b/dbms/src/Storages/StorageMergeTree.h
@@ -79,7 +79,7 @@ class StorageMergeTree : public ext::shared_ptr_helper<StorageMergeTree>, public
 
     void alter(const AlterCommands & params, const String & database_name, const String & table_name, const Context & context) override;
 
-    void alterForTMT(const AlterCommands & params, const TiDB::TableInfo & table_info, const Context & context);
+    void alterForTMT(const AlterCommands & params, const TiDB::TableInfo & table_info, const String & database_name, const Context & context);
 
     bool checkTableCanBeDropped() const override;
 

diff --git a/dbms/src/Storages/Transaction/SchemaBuilder.cpp b/dbms/src/Storages/Transaction/SchemaBuilder.cpp
@@ -20,7 +20,7 @@ using DBInfo = TiDB::DBInfo;
 using TableInfoPtr = TiDB::TableInfoPtr;
 using DBInfoPtr = TiDB::DBInfoPtr;
 
-inline AlterCommands detectSchemaChanges(const TiDB::TableInfo & table_info, const TiDB::TableInfo & orig_table_info)
+inline AlterCommands detectSchemaChanges(Logger * log, const TiDB::TableInfo & table_info, const TiDB::TableInfo & orig_table_info)
 {
     AlterCommands alter_commands;
 
@@ -39,8 +39,8 @@ inline AlterCommands detectSchemaChanges(const TiDB::TableInfo & table_info, con
             command.type = AlterCommand::ADD_COLUMN;
             command.column_name = column_info.name;
             command.data_type = getDataTypeByColumnInfo(column_info);
-            // TODO: support default value.
             // TODO: support after column.
+            LOG_DEBUG(log, "detect add column.");
         }
         else
         {
@@ -77,20 +77,20 @@ inline AlterCommands detectSchemaChanges(const TiDB::TableInfo & table_info, con
     return alter_commands;
 }
 
-void SchemaBuilder::applyAlterTableImpl(TiDB::TableInfoPtr table_info, StorageMergeTree * storage)
+void SchemaBuilder::applyAlterTableImpl(TiDB::TableInfoPtr table_info, const String & db_name, StorageMergeTree * storage)
 {
     auto orig_table_info = storage->getTableInfo();
-    auto commands = detectSchemaChanges(*table_info, orig_table_info);
+    auto commands = detectSchemaChanges(log, *table_info, orig_table_info);
 
-    storage->alterForTMT(commands, *table_info, context);
+    storage->alterForTMT(commands, *table_info, db_name, context);
 
     if (table_info->is_partition_table)
     {
         // create partition table.
         for (auto part_def : table_info->partition.definitions)
         {
             auto new_table_info = table_info->producePartitionTableInfo(part_def.id);
-            storage->alterForTMT(commands, *new_table_info, context);
+            storage->alterForTMT(commands, *new_table_info, db_name, context);
         }
     }
 }
@@ -103,9 +103,9 @@ void SchemaBuilder::applyAlterTable(TiDB::DBInfoPtr dbInfo, Int64 table_id)
     auto storage = static_cast<StorageMergeTree *>(tmt_context.getStorages().get(table_id).get());
     if (storage == nullptr)
     {
-        // TODO throw exception
+        throw Exception("miss table: " + std::to_string(table_id));
     }
-    applyAlterTableImpl(table_info, storage);
+    applyAlterTableImpl(table_info, dbInfo->name, storage);
 }
 
 void SchemaBuilder::applyDiff(const SchemaDiff & diff)
@@ -126,7 +126,7 @@ void SchemaBuilder::applyDiff(const SchemaDiff & diff)
     auto di = getter.getDatabase(diff.schema_id);
 
     if (di == nullptr)
-        throw Exception("miss database: ", std::to_string(diff.schema_id));
+        throw Exception("miss database: " + std::to_string(diff.schema_id));
 
     Int64 oldTableID = 0, newTableID = 0;
 
@@ -370,7 +370,7 @@ void SchemaBuilder::updateDB(TiDB::DBInfoPtr db_info)
         }
         else
         {
-            applyAlterTableImpl(table, storage);
+            applyAlterTableImpl(table, db_info->name, storage);
         }
     }
 }

diff --git a/dbms/src/Storages/Transaction/SchemaBuilder.h b/dbms/src/Storages/Transaction/SchemaBuilder.h
@@ -11,11 +11,15 @@ struct SchemaBuilder
 {
 
     SchemaGetter & getter;
+
     Context & context;
+
     std::unordered_map<DB::DatabaseID, String> & databases;
 
+    Logger * log;
+
     SchemaBuilder(SchemaGetter & getter_, Context & context_, std::unordered_map<DB::DatabaseID, String> & dbs_)
-        : getter(getter_), context(context_), databases(dbs_)
+        : getter(getter_), context(context_), databases(dbs_), log(&Logger::get("SchemaBuilder"))
     {}
 
     void applyDiff(const SchemaDiff & diff);
@@ -36,7 +40,7 @@ struct SchemaBuilder
 
     void applyAlterTable(TiDB::DBInfoPtr dbInfo, Int64 table_id);
 
-    void applyAlterTableImpl(TiDB::TableInfoPtr table_info, StorageMergeTree * storage);
+    void applyAlterTableImpl(TiDB::TableInfoPtr table_info, const String & db_name, StorageMergeTree * storage);
 
     //void applyAddPartition(TiDB::DBInfoPtr dbInfo, Int64 table_id);