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.

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-835]fix bugs that rename column after adding a column #396

Merged
merged 5 commits into from
Jan 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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()