Merge pull request #50154 from hanfei1991/hanfei/fix-modify-order-by

do not allow modify order by when there are no order by cols
This commit is contained in:
alesapin 2023-05-24 15:01:38 +02:00 committed by GitHub
commit 7c0c49c9d2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 41 additions and 39 deletions

View File

@ -506,52 +506,48 @@ void MergeTreeData::checkProperties(
auto all_columns = new_metadata.columns.getAllPhysical();
/// Order by check AST
if (old_metadata.hasSortingKey())
/// This is ALTER, not CREATE/ATTACH TABLE. Let us check that all new columns used in the sorting key
/// expression have just been added (so that the sorting order is guaranteed to be valid with the new key).
Names new_primary_key_columns = new_primary_key.column_names;
Names new_sorting_key_columns = new_sorting_key.column_names;
ASTPtr added_key_column_expr_list = std::make_shared<ASTExpressionList>();
const auto & old_sorting_key_columns = old_metadata.getSortingKeyColumns();
for (size_t new_i = 0, old_i = 0; new_i < sorting_key_size; ++new_i)
{
/// This is ALTER, not CREATE/ATTACH TABLE. Let us check that all new columns used in the sorting key
/// expression have just been added (so that the sorting order is guaranteed to be valid with the new key).
Names new_primary_key_columns = new_primary_key.column_names;
Names new_sorting_key_columns = new_sorting_key.column_names;
ASTPtr added_key_column_expr_list = std::make_shared<ASTExpressionList>();
const auto & old_sorting_key_columns = old_metadata.getSortingKeyColumns();
for (size_t new_i = 0, old_i = 0; new_i < sorting_key_size; ++new_i)
if (old_i < old_sorting_key_columns.size())
{
if (old_i < old_sorting_key_columns.size())
{
if (new_sorting_key_columns[new_i] != old_sorting_key_columns[old_i])
added_key_column_expr_list->children.push_back(new_sorting_key.expression_list_ast->children[new_i]);
else
++old_i;
}
else
if (new_sorting_key_columns[new_i] != old_sorting_key_columns[old_i])
added_key_column_expr_list->children.push_back(new_sorting_key.expression_list_ast->children[new_i]);
else
++old_i;
}
else
added_key_column_expr_list->children.push_back(new_sorting_key.expression_list_ast->children[new_i]);
}
if (!added_key_column_expr_list->children.empty())
if (!added_key_column_expr_list->children.empty())
{
auto syntax = TreeRewriter(getContext()).analyze(added_key_column_expr_list, all_columns);
Names used_columns = syntax->requiredSourceColumns();
NamesAndTypesList deleted_columns;
NamesAndTypesList added_columns;
old_metadata.getColumns().getAllPhysical().getDifference(all_columns, deleted_columns, added_columns);
for (const String & col : used_columns)
{
auto syntax = TreeRewriter(getContext()).analyze(added_key_column_expr_list, all_columns);
Names used_columns = syntax->requiredSourceColumns();
if (!added_columns.contains(col) || deleted_columns.contains(col))
throw Exception(ErrorCodes::BAD_ARGUMENTS,
"Existing column {} is used in the expression that was added to the sorting key. "
"You can add expressions that use only the newly added columns",
backQuoteIfNeed(col));
NamesAndTypesList deleted_columns;
NamesAndTypesList added_columns;
old_metadata.getColumns().getAllPhysical().getDifference(all_columns, deleted_columns, added_columns);
for (const String & col : used_columns)
{
if (!added_columns.contains(col) || deleted_columns.contains(col))
throw Exception(ErrorCodes::BAD_ARGUMENTS,
"Existing column {} is used in the expression that was added to the sorting key. "
"You can add expressions that use only the newly added columns",
backQuoteIfNeed(col));
if (new_metadata.columns.getDefaults().contains(col))
throw Exception(ErrorCodes::BAD_ARGUMENTS,
"Newly added column {} has a default expression, so adding expressions that use "
"it to the sorting key is forbidden", backQuoteIfNeed(col));
}
if (new_metadata.columns.getDefaults().contains(col))
throw Exception(ErrorCodes::BAD_ARGUMENTS,
"Newly added column {} has a default expression, so adding expressions that use "
"it to the sorting key is forbidden", backQuoteIfNeed(col));
}
}

View File

@ -1,6 +1,12 @@
SET send_logs_level = 'fatal';
SET optimize_on_insert = 0;
DROP TABLE IF EXISTS no_order;
CREATE TABLE no_order(a UInt32, b UInt32) ENGINE = MergeTree ORDER BY tuple();
ALTER TABLE no_order MODIFY ORDER BY (a); -- { serverError 36}
DROP TABLE no_order;
DROP TABLE IF EXISTS old_style;
set allow_deprecated_syntax_for_merge_tree=1;
CREATE TABLE old_style(d Date, x UInt32) ENGINE MergeTree(d, x, 8192);