From 9314f91a24eb09081531be1c12280cc1ea521243 Mon Sep 17 00:00:00 2001 From: Han Fei Date: Tue, 16 Jul 2019 19:23:01 +0800 Subject: [PATCH] fix bug during creating column: use a wrong db name (#107) --- dbms/src/Storages/StorageMergeTree.cpp | 2 +- dbms/src/Storages/StorageMergeTree.h | 2 +- .../Storages/Transaction/SchemaBuilder.cpp | 20 +++++++++---------- dbms/src/Storages/Transaction/SchemaBuilder.h | 8 ++++++-- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/dbms/src/Storages/StorageMergeTree.cpp b/dbms/src/Storages/StorageMergeTree.cpp index 3a8bc27a95f..c8f90d96c74 100644 --- 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 index 81a8e038cb4..8582e8232d5 100644 --- a/dbms/src/Storages/StorageMergeTree.h +++ b/dbms/src/Storages/StorageMergeTree.h @@ -79,7 +79,7 @@ class StorageMergeTree : public ext::shared_ptr_helper, 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 index 97e64f9c945..92243b48323 100644 --- 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,12 +77,12 @@ 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) { @@ -90,7 +90,7 @@ void SchemaBuilder::applyAlterTableImpl(TiDB::TableInfoPtr table_info, StorageMe 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(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 index 2435bb433f5..f78bd3939ef 100644 --- 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 & databases; + Logger * log; + SchemaBuilder(SchemaGetter & getter_, Context & context_, std::unordered_map & 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);