diff --git a/dbms/src/Debug/MockTiDB.cpp b/dbms/src/Debug/MockTiDB.cpp index 2e7822a8bbf..d777820c6fa 100644 --- a/dbms/src/Debug/MockTiDB.cpp +++ b/dbms/src/Debug/MockTiDB.cpp @@ -25,7 +25,7 @@ using Table = MockTiDB::Table; using TablePtr = MockTiDB::TablePtr; Table::Table(const String & database_name_, const String & table_name_, TableInfo && table_info_) - : table_info(std::move(table_info_)), database_name(database_name_), table_name(table_name_) + : table_info(std::move(table_info_)), database_name(database_name_), table_name(table_name_), col_id(table_info_.columns.size()) {} MockTiDB::MockTiDB() { databases["default"] = 0; } @@ -325,7 +325,7 @@ void MockTiDB::addColumnToTable(const String & database_name, const String & tab != columns.end()) throw Exception("Column " + column.name + " already exists in TiDB table " + qualified_name, ErrorCodes::LOGICAL_ERROR); - ColumnInfo column_info = getColumnInfoFromColumn(column, columns.back().id + 1); + ColumnInfo column_info = getColumnInfoFromColumn(column, table->allocColumnID()); columns.emplace_back(column_info); version++; diff --git a/dbms/src/Debug/MockTiDB.h b/dbms/src/Debug/MockTiDB.h index 2ea55380a05..d2b8e6eeb33 100644 --- a/dbms/src/Debug/MockTiDB.h +++ b/dbms/src/Debug/MockTiDB.h @@ -33,6 +33,8 @@ class MockTiDB : public ext::singleton TableID id() { return table_info.id; } + ColumnID allocColumnID() { return ++col_id; } + bool isPartitionTable() { return table_info.is_partition_table; } std::vector getPartitionIDs() @@ -48,6 +50,7 @@ class MockTiDB : public ext::singleton private: const String database_name; const String table_name; + ColumnID col_id; }; using TablePtr = std::shared_ptr; diff --git a/dbms/src/Storages/Transaction/SchemaBuilder.cpp b/dbms/src/Storages/Transaction/SchemaBuilder.cpp index f0196f4247f..8dcdfe00117 100644 --- a/dbms/src/Storages/Transaction/SchemaBuilder.cpp +++ b/dbms/src/Storages/Transaction/SchemaBuilder.cpp @@ -43,18 +43,20 @@ inline AlterCommands detectSchemaChanges(Logger * log, const TableInfo & table_i /// Detect new columns. // TODO: Detect rename columns. - for (const auto & column_info : table_info.columns) + + /// Detect dropped columns. + for (const auto & orig_column_info : orig_table_info.columns) { - const auto & orig_column_info = std::find_if(orig_table_info.columns.begin(), - orig_table_info.columns.end(), - [&](const TiDB::ColumnInfo & orig_column_info_) { return orig_column_info_.id == column_info.id; }); + const auto & column_info = std::find_if(table_info.columns.begin(), + table_info.columns.end(), + [&](const TiDB::ColumnInfo & column_info_) { return column_info_.id == orig_column_info.id; }); AlterCommand command; - if (orig_column_info == orig_table_info.columns.end()) + if (column_info == table_info.columns.end()) { - // New column. - command.type = AlterCommand::ADD_COLUMN; - setAlterCommandColumn(log, command, column_info); + // Dropped column. + command.type = AlterCommand::DROP_COLUMN; + command.column_name = orig_column_info.name; } else { @@ -65,19 +67,18 @@ inline AlterCommands detectSchemaChanges(Logger * log, const TableInfo & table_i alter_commands.emplace_back(std::move(command)); } - /// Detect dropped columns. - for (const auto & orig_column_info : orig_table_info.columns) + for (const auto & column_info : table_info.columns) { - const auto & column_info = std::find_if(table_info.columns.begin(), - table_info.columns.end(), - [&](const TiDB::ColumnInfo & column_info_) { return column_info_.id == orig_column_info.id; }); + const auto & orig_column_info = std::find_if(orig_table_info.columns.begin(), + orig_table_info.columns.end(), + [&](const TiDB::ColumnInfo & orig_column_info_) { return orig_column_info_.id == column_info.id; }); AlterCommand command; - if (column_info == table_info.columns.end()) + if (orig_column_info == orig_table_info.columns.end()) { - // Dropped column. - command.type = AlterCommand::DROP_COLUMN; - command.column_name = orig_column_info.name; + // New column. + command.type = AlterCommand::ADD_COLUMN; + setAlterCommandColumn(log, command, column_info); } else { diff --git a/tests/mutable-test/txn_schema/alter_on_read.test b/tests/mutable-test/txn_schema/alter_on_read.test index 1bd8b82102f..583df2e92a4 100644 --- a/tests/mutable-test/txn_schema/alter_on_read.test +++ b/tests/mutable-test/txn_schema/alter_on_read.test @@ -164,7 +164,39 @@ Code: 49. DB::Exception: Received from {#WORD} DB::Exception: Detected overflow => select col_2 from default.test Received exception from server (version {#WORD}): Code: 47. DB::Exception: Received from {#WORD} DB::Exception: Unknown identifier: col_2. +=> DBGInvoke __add_column_to_tidb_table(default, test, 'col_2 Nullable(Int8)') +=> DBGInvoke __refresh_schemas() +=> select col_2 from default.test +┌─col_2─┐ +│ \N │ +│ \N │ +│ \N │ +│ \N │ +│ \N │ +│ \N │ +│ \N │ +│ \N │ +│ \N │ +└───────┘ +=> DBGInvoke __reset_schemas() +=> DBGInvoke __drop_column_from_tidb_table(default, test, col_2) + +=> DBGInvoke __add_column_to_tidb_table(default, test, 'col_2 Int8') +=> DBGInvoke __refresh_schemas() + +=> select col_2 from default.test +┌─col_2─┐ +│ 0 │ +│ 0 │ +│ 0 │ +│ 0 │ +│ 0 │ +│ 0 │ +│ 0 │ +│ 0 │ +│ 0 │ +└───────┘ # Clean up. => DBGInvoke __drop_tidb_table(default, test) => drop table if exists default.test