Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flash-443] fix drop and add column with same name #193

Merged
merged 2 commits into from
Aug 23, 2019
Merged

[flash-443] fix drop and add column with same name #193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
30db000
[flash-443] fix drop and add column with same name
hanfei1991 Aug 22, 2019
23aaaf3
Merge branch 'master' into hanfei-ddl/fix_add_drop_col
hanfei1991 Aug 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions dbms/src/Debug/MockTiDB.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down Expand Up @@ -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++;
Expand Down
3 changes: 3 additions & 0 deletions dbms/src/Debug/MockTiDB.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class MockTiDB : public ext::singleton<MockTiDB>

TableID id() { return table_info.id; }

ColumnID allocColumnID() { return ++col_id; }

bool isPartitionTable() { return table_info.is_partition_table; }

std::vector<TableID> getPartitionIDs()
Expand All @@ -48,6 +50,7 @@ class MockTiDB : public ext::singleton<MockTiDB>
private:
const String database_name;
const String table_name;
ColumnID col_id;
};
using TablePtr = std::shared_ptr<Table>;

Expand Down
35 changes: 18 additions & 17 deletions dbms/src/Storages/Transaction/SchemaBuilder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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
{
Expand Down
32 changes: 32 additions & 0 deletions tests/mutable-test/txn_schema/alter_on_read.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down