Skip to content

Commit

Permalink
[flash-835]fix bugs that rename column after adding a column (#396)
Browse files Browse the repository at this point in the history
* [flash-835]fix bugs that rename column after adding a column

* address comment
  • Loading branch information
hanfei1991 authored Jan 19, 2020
1 parent 65f0b55 commit ccd9324
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 1 deletion.
8 changes: 8 additions & 0 deletions dbms/src/Storages/AlterCommands.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,15 @@ void AlterCommand::apply(ColumnsDescription & columns_description) const
{
throw Exception("Rename column fails, new column name: " + column_name + " has existed ", ErrorCodes::LOGICAL_ERROR);
}
String old_column_name = old_column_it->name;
old_column_it->name = new_column_name;
// Change defaults value map.
auto default_it = columns_description.defaults.find(old_column_name);
if (default_it != columns_description.defaults.end())
{
columns_description.defaults.erase(default_it->first);
columns_description.defaults.emplace(new_column_name, default_it->second);
}
}
else
throw Exception("Wrong parameter type in ALTER query", ErrorCodes::LOGICAL_ERROR);
Expand Down
14 changes: 14 additions & 0 deletions dbms/src/Storages/MergeTree/MergeTreeData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1132,6 +1132,20 @@ MergeTreeData::AlterDataPartTransactionPtr MergeTreeData::renameColumnPart(
{
throw Exception("L0 compact does not support alter clause.");
}

// Check if this part has column that needs to be renamed.
// There is one case that column cannot be found : the column is added after the table is created.
auto it = std::find_if(part->columns.begin(), part->columns.end(), [&](const NameAndTypePair & pair)
{
return pair.name == command.column_name;
});

if (it == part->columns.end())
{
LOG_INFO(log, "Not find column name " << command.column_name << " in part " << part->name);
return nullptr;
}

AlterDataPartTransactionPtr transaction(new AlterDataPartTransaction(part)); /// Blocks changes to the part.
DataPart::Checksums new_checksums = part->checksums;

Expand Down
3 changes: 2 additions & 1 deletion dbms/src/Storages/StorageMergeTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,8 @@ void StorageMergeTree::alterInternal(
if (rename_column)
{
auto transaction = data.renameColumnPart(part, columns_for_parts, params[0]);
transactions.push_back(std::move(transaction));
if (transaction != nullptr)
transactions.push_back(std::move(transaction));
}
else if (auto transaction = data.alterDataPart(part, columns_for_parts, new_primary_key_ast, false))
transactions.push_back(std::move(transaction));
Expand Down
11 changes: 11 additions & 0 deletions tests/mutable-test/txn_schema/rename_column.test
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,17 @@
│ 2 │ test │
└───┴──────┘

# rename after add column
=> DBGInvoke __add_column_to_tidb_table(default, test, 'k Nullable(Int8) default 0')
=> DBGInvoke __refresh_schemas()
=> DBGInvoke __rename_column_in_tidb_table(default, test, k, g)
=> DBGInvoke __refresh_schemas()
=> select a, g from default.test order by _tidb_rowid
┌─a─┬─g─┐
│ 1 │ 0 │
│ 2 │ 0 │
└───┴───┘

=> DBGInvoke __drop_tidb_table(default, test)
=> drop table if exists default.test
=> DBGInvoke __refresh_schemas()

0 comments on commit ccd9324

Please sign in to comment.