mirror of
https://github.com/ClickHouse/ClickHouse.git
synced 2024-11-26 01:22:04 +00:00
Less strict check and rare rename bug
This commit is contained in:
parent
2d8e36ed5f
commit
1707f84a44
@ -777,10 +777,10 @@ bool MutationsInterpreter::Stage::isAffectingAllColumns(const Names & storage_co
|
||||
bool MutationsInterpreter::isAffectingAllColumns() const
|
||||
{
|
||||
auto storage_columns = metadata_snapshot->getColumns().getNamesOfPhysical();
|
||||
for (const auto & stage : stages)
|
||||
if (stage.isAffectingAllColumns(storage_columns))
|
||||
return true;
|
||||
return false;
|
||||
if (stages.empty())
|
||||
throw Exception(ErrorCodes::LOGICAL_ERROR, "Mutation interpreter has no stages");
|
||||
|
||||
return stages.back().isAffectingAllColumns(storage_columns);
|
||||
}
|
||||
|
||||
}
|
||||
|
@ -42,7 +42,7 @@ public:
|
||||
/// Only changed columns.
|
||||
const Block & getUpdatedHeader() const;
|
||||
|
||||
/// At least one mutation stage affects all columns in storage
|
||||
/// Latest mutation stage affects all columns in storage
|
||||
bool isAffectingAllColumns() const;
|
||||
|
||||
private:
|
||||
|
@ -557,7 +557,8 @@ void IMergeTreeDataPart::loadRowsCount()
|
||||
/// columns have to be loaded
|
||||
for (const auto & column : getColumns())
|
||||
{
|
||||
if (column.type->isValueRepresentedByNumber())
|
||||
/// Most trivial types
|
||||
if (column.type->isValueRepresentedByNumber() && !column.type->haveSubtypes())
|
||||
{
|
||||
auto size = getColumnSize(column.name, *column.type);
|
||||
|
||||
|
@ -1478,13 +1478,14 @@ NamesAndTypesList MergeTreeDataMergerMutator::getColumnsForNewDataPart(
|
||||
return updated_header.getNamesAndTypesList();
|
||||
|
||||
NameSet removed_columns;
|
||||
NameToNameMap renamed_columns;
|
||||
NameToNameMap renamed_columns_to_from;
|
||||
/// All commands are validated in AlterCommand so we don't care about order
|
||||
for (const auto & command : commands_for_removes)
|
||||
{
|
||||
if (command.type == MutationCommand::DROP_COLUMN)
|
||||
removed_columns.insert(command.column_name);
|
||||
if (command.type == MutationCommand::RENAME_COLUMN)
|
||||
renamed_columns.emplace(command.rename_to, command.column_name);
|
||||
renamed_columns_to_from.emplace(command.rename_to, command.column_name);
|
||||
}
|
||||
Names source_column_names = source_part->getColumns().getNames();
|
||||
NameSet source_columns_name_set(source_column_names.begin(), source_column_names.end());
|
||||
@ -1497,17 +1498,49 @@ NamesAndTypesList MergeTreeDataMergerMutator::getColumnsForNewDataPart(
|
||||
it->type = updated_type;
|
||||
++it;
|
||||
}
|
||||
else if (source_columns_name_set.count(it->name) && !removed_columns.count(it->name))
|
||||
else
|
||||
{
|
||||
++it;
|
||||
}
|
||||
else if (renamed_columns.count(it->name) && source_columns_name_set.count(renamed_columns[it->name]))
|
||||
if (!source_columns_name_set.count(it->name))
|
||||
{
|
||||
/// Source part doesn't have column but some other column
|
||||
/// was renamed to it's name.
|
||||
auto renamed_it = renamed_columns_to_from.find(it->name);
|
||||
if (renamed_it != renamed_columns_to_from.end()
|
||||
&& source_columns_name_set.count(renamed_it->second))
|
||||
++it;
|
||||
else
|
||||
it = storage_columns.erase(it);
|
||||
}
|
||||
else
|
||||
{
|
||||
bool was_renamed = false;
|
||||
bool was_removed = removed_columns.count(it->name);
|
||||
|
||||
/// Check that this column was renamed to some other name
|
||||
for (const auto & [rename_to, rename_from] : renamed_columns_to_from)
|
||||
{
|
||||
if (rename_from == it->name)
|
||||
{
|
||||
was_renamed = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
/// If we want to rename this column to some other name, than it
|
||||
/// should it's previous version should be dropped or removed
|
||||
if (renamed_columns_to_from.count(it->name) && !was_renamed && !was_removed)
|
||||
throw Exception(
|
||||
ErrorCodes::LOGICAL_ERROR,
|
||||
"Incorrect mutation commands, trying to rename column {} to {}, but part {} already has column {}", renamed_columns_to_from[it->name], it->name, source_part->name, it->name);
|
||||
|
||||
|
||||
/// Column was renamed and no other column renamed to it's name
|
||||
/// or column is dropped.
|
||||
if (!renamed_columns_to_from.count(it->name) && (was_renamed || was_removed))
|
||||
it = storage_columns.erase(it);
|
||||
else
|
||||
++it;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -240,7 +240,8 @@ void MergeTreeDataPartWide::calculateEachColumnSizes(ColumnSizeByName & each_col
|
||||
total_size.add(size);
|
||||
|
||||
#ifndef NDEBUG
|
||||
if (rows_count != 0 && column.type->isValueRepresentedByNumber())
|
||||
/// Most trivial types
|
||||
if (rows_count != 0 && column.type->isValueRepresentedByNumber() && !column.type->haveSubtypes())
|
||||
{
|
||||
size_t rows_in_column = size.data_uncompressed / column.type->getSizeOfValueInMemory();
|
||||
if (rows_in_column != rows_count)
|
||||
|
Loading…
Reference in New Issue
Block a user