Merge pull request #35884 from ClickHouse/don_t_allow_to_reset_settings

Forbid to reset non existing settings
This commit is contained in:
alesapin 2022-04-04 15:27:34 +02:00 committed by GitHub
commit 3ccf99c3d7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 27 additions and 5 deletions

View File

@ -1909,6 +1909,7 @@ void MergeTreeData::checkAlterIsPossible(const AlterCommands & commands, Context
StorageInMemoryMetadata old_metadata = getInMemoryMetadata(); StorageInMemoryMetadata old_metadata = getInMemoryMetadata();
const auto & settings = local_context->getSettingsRef(); const auto & settings = local_context->getSettingsRef();
const auto & settings_from_storage = getSettings();
if (!settings.allow_non_metadata_alters) if (!settings.allow_non_metadata_alters)
{ {
@ -2099,6 +2100,14 @@ void MergeTreeData::checkAlterIsPossible(const AlterCommands & commands, Context
dropped_columns.emplace(command.column_name); dropped_columns.emplace(command.column_name);
} }
else if (command.type == AlterCommand::RESET_SETTING)
{
for (const auto & reset_setting : command.settings_resets)
{
if (!settings_from_storage->has(reset_setting))
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Cannot reset setting '{}' because it doesn't exist for MergeTree engines family", reset_setting);
}
}
else if (command.isRequireMutationStage(getInMemoryMetadata())) else if (command.isRequireMutationStage(getInMemoryMetadata()))
{ {
/// This alter will override data on disk. Let's check that it doesn't /// This alter will override data on disk. Let's check that it doesn't

View File

@ -91,8 +91,8 @@ SHOW CREATE TABLE table_for_reset_setting;
ALTER TABLE table_for_reset_setting RESET SETTING index_granularity; -- { serverError 472 } ALTER TABLE table_for_reset_setting RESET SETTING index_granularity; -- { serverError 472 }
-- ignore undefined setting -- don't execute alter with incorrect setting
ALTER TABLE table_for_reset_setting RESET SETTING merge_with_ttl_timeout, unknown_setting; ALTER TABLE table_for_reset_setting RESET SETTING merge_with_ttl_timeout, unknown_setting; -- { serverError 36 }
ALTER TABLE table_for_reset_setting MODIFY SETTING merge_with_ttl_timeout = 300, max_concurrent_queries = 1; ALTER TABLE table_for_reset_setting MODIFY SETTING merge_with_ttl_timeout = 300, max_concurrent_queries = 1;
@ -102,4 +102,4 @@ ALTER TABLE table_for_reset_setting RESET SETTING max_concurrent_queries, merge_
SHOW CREATE TABLE table_for_reset_setting; SHOW CREATE TABLE table_for_reset_setting;
DROP TABLE IF EXISTS table_for_reset_setting; DROP TABLE IF EXISTS table_for_reset_setting;

View File

@ -108,8 +108,8 @@ ATTACH TABLE replicated_table_for_reset_setting1;
SHOW CREATE TABLE replicated_table_for_reset_setting1; SHOW CREATE TABLE replicated_table_for_reset_setting1;
SHOW CREATE TABLE replicated_table_for_reset_setting2; SHOW CREATE TABLE replicated_table_for_reset_setting2;
-- ignore undefined setting -- don't execute alter with incorrect setting
ALTER TABLE replicated_table_for_reset_setting1 RESET SETTING check_delay_period, unknown_setting; ALTER TABLE replicated_table_for_reset_setting1 RESET SETTING check_delay_period, unknown_setting; -- { serverError 36 }
ALTER TABLE replicated_table_for_reset_setting1 RESET SETTING merge_with_ttl_timeout; ALTER TABLE replicated_table_for_reset_setting1 RESET SETTING merge_with_ttl_timeout;
ALTER TABLE replicated_table_for_reset_setting2 RESET SETTING merge_with_ttl_timeout; ALTER TABLE replicated_table_for_reset_setting2 RESET SETTING merge_with_ttl_timeout;

View File

@ -0,0 +1,13 @@
DROP TABLE IF EXISTS most_ordinary_mt;
CREATE TABLE most_ordinary_mt
(
Key UInt64
)
ENGINE = MergeTree()
ORDER BY tuple();
ALTER TABLE most_ordinary_mt RESET SETTING ttl; --{serverError 36}
ALTER TABLE most_ordinary_mt RESET SETTING allow_remote_fs_zero_copy_replication, xxx; --{serverError 36}
DROP TABLE IF EXISTS most_ordinary_mt;