From d20b3d78c5b8faf673076165faf4e2c8e3676b2c Mon Sep 17 00:00:00 2001 From: Vitaly Baranov Date: Tue, 12 Apr 2022 22:38:21 +0200 Subject: [PATCH] Rename some restore settings. --- src/Backups/RestoreSettings.cpp | 42 +++++++++++++++---- src/Backups/RestoreSettings.h | 37 +++++++++++----- src/Backups/RestoreUtils.cpp | 25 ++++++----- src/Common/ErrorCodes.cpp | 1 + .../test_backup_restore_new/test.py | 4 +- 5 files changed, 78 insertions(+), 31 deletions(-) diff --git a/src/Backups/RestoreSettings.cpp b/src/Backups/RestoreSettings.cpp index 485650e39f0..b1d3c157e13 100644 --- a/src/Backups/RestoreSettings.cpp +++ b/src/Backups/RestoreSettings.cpp @@ -1,8 +1,10 @@ #include #include +#include #include #include #include +#include namespace DB @@ -10,6 +12,30 @@ namespace DB namespace ErrorCodes { extern const int UNKNOWN_SETTING; + extern const int CANNOT_PARSE_RESTORE_TABLE_CREATION_MODE; +} + +namespace +{ + RestoreTableCreationMode parseRestoreTableCreationMode(const Field & field) + { + if (field.getType() == Field::Types::String) + { + String str = field.get(); + if (str == "1" || boost::iequals(str, "true")) + return RestoreTableCreationMode::kCreate; + if (str == "0" || boost::iequals(str, "false")) + return RestoreTableCreationMode::kMustExist; + if (boost::iequals(str, "if not exists")) + return RestoreTableCreationMode::kCreateIfNotExists; + throw Exception("Cannot parse creation mode from string '" + str + "'", + ErrorCodes::CANNOT_PARSE_RESTORE_TABLE_CREATION_MODE); + } + if (applyVisitor(FieldVisitorConvertToNumber(), field)) + return RestoreTableCreationMode::kCreate; + else + return RestoreTableCreationMode::kMustExist; + } } RestoreSettings RestoreSettings::fromRestoreQuery(const ASTBackupQuery & query) @@ -28,14 +54,14 @@ RestoreSettings RestoreSettings::fromRestoreQuery(const ASTBackupQuery & query) res.password = SettingFieldString{setting.value}; else if (setting.name == "structure_only") res.structure_only = SettingFieldBool{setting.value}; - else if (setting.name == "throw_if_database_exists") - res.throw_if_database_exists = SettingFieldBool{setting.value}; - else if (setting.name == "throw_if_table_exists") - res.throw_if_table_exists = SettingFieldBool{setting.value}; - else if (setting.name == "throw_if_database_def_differs") - res.throw_if_database_def_differs = SettingFieldBool{setting.value}; - else if (setting.name == "throw_if_table_def_differs") - res.throw_if_table_def_differs = SettingFieldBool{setting.value}; + else if (setting.name == "create_table") + res.create_table = parseRestoreTableCreationMode(setting.value); + else if (setting.name == "create_database") + res.create_database = parseRestoreTableCreationMode(setting.value); + else if (setting.name == "allow_different_table_def") + res.allow_different_table_def = SettingFieldBool{setting.value}; + else if (setting.name == "allow_different_database_def") + res.allow_different_database_def = SettingFieldBool{setting.value}; else throw Exception(ErrorCodes::UNKNOWN_SETTING, "Unknown setting {}", setting.name); } diff --git a/src/Backups/RestoreSettings.h b/src/Backups/RestoreSettings.h index b129224943b..9f4951862c7 100644 --- a/src/Backups/RestoreSettings.h +++ b/src/Backups/RestoreSettings.h @@ -12,6 +12,21 @@ struct StorageRestoreSettings { }; +/// How the RESTORE command will handle table/database existence. +enum class RestoreTableCreationMode +{ + /// RESTORE TABLE always tries to create a table and it throws an exception if the table already exists. + kCreate, + + /// RESTORE TABLE never tries to create a table and it throws an exception if the table doesn't exist. + kMustExist, + + /// RESTORE TABLE tries to create a table if it doesn't exist. + kCreateIfNotExists, +}; + +using RestoreDatabaseCreationMode = RestoreTableCreationMode; + /// Settings specified in the "SETTINGS" clause of a RESTORE query. struct RestoreSettings : public StorageRestoreSettings { @@ -27,19 +42,21 @@ struct RestoreSettings : public StorageRestoreSettings /// without the data of tables. bool structure_only = false; - /// Whether RESTORE DATABASE must throw an exception if a destination database already exists. - bool throw_if_database_exists = true; + /// How RESTORE command should work if a table to restore already exists. + RestoreTableCreationMode create_table = RestoreTableCreationMode::kCreateIfNotExists; - /// Whether RESTORE TABLE must throw an exception if a destination table already exists. - bool throw_if_table_exists = true; + /// How RESTORE command should work if a database to restore already exists. + RestoreDatabaseCreationMode create_database = RestoreDatabaseCreationMode::kCreateIfNotExists; - /// Whether RESTORE DATABASE must throw an exception if a destination database has - /// a different definition comparing with the definition read from backup. - bool throw_if_database_def_differs = true; + /// Normally RESTORE command throws an exception if a destination table exists but has a different definition + /// (i.e. create query) comparing with its definition extracted from backup. + /// Set `allow_different_table_def` to true to skip this check. + bool allow_different_table_def = false; - /// Whether RESTORE TABLE must throw an exception if a destination table has - /// a different definition comparing with the definition read from backup. - bool throw_if_table_def_differs = true; + /// Normally RESTORE command throws an exception if a destination database exists but has a different definition + /// (i.e. create query) comparing with its definition extracted from backup. + /// Set `allow_different_database_def` to true to skip this check. + bool allow_different_database_def = false; static RestoreSettings fromRestoreQuery(const ASTBackupQuery & query); }; diff --git a/src/Backups/RestoreUtils.cpp b/src/Backups/RestoreUtils.cpp index 8073b6d0818..f439860e61c 100644 --- a/src/Backups/RestoreUtils.cpp +++ b/src/Backups/RestoreUtils.cpp @@ -70,9 +70,12 @@ namespace private: void createDatabase() { - /// We need to call clone() for `create_query` because the interpreter can decide - /// to change a passed AST a little bit. - InterpreterCreateQuery create_interpreter{create_query->clone(), context}; + if (restore_settings->create_database == RestoreDatabaseCreationMode::kMustExist) + return; + + auto cloned_create_query = typeid_cast>(create_query->clone()); + cloned_create_query->if_not_exists = (restore_settings->create_database == RestoreDatabaseCreationMode::kCreateIfNotExists); + InterpreterCreateQuery create_interpreter{cloned_create_query, context}; create_interpreter.execute(); } @@ -92,7 +95,7 @@ namespace void checkDatabaseCreateQuery() { - if (ignore_if_database_def_differs || !restore_settings->throw_if_database_def_differs) + if (ignore_if_database_def_differs || restore_settings->allow_different_database_def) return; getDatabaseCreateQuery(); @@ -153,9 +156,12 @@ namespace private: void createStorage() { - /// We need to call clone() for `create_query` because the interpreter can decide - /// to change a passed AST a little bit. - InterpreterCreateQuery create_interpreter{create_query->clone(), context}; + if (restore_settings->create_table == RestoreTableCreationMode::kMustExist) + return; + + auto cloned_create_query = typeid_cast>(create_query->clone()); + cloned_create_query->if_not_exists = (restore_settings->create_table == RestoreTableCreationMode::kCreateIfNotExists); + InterpreterCreateQuery create_interpreter{cloned_create_query, context}; create_interpreter.execute(); } @@ -178,7 +184,7 @@ namespace void checkStorageCreateQuery() { - if (!restore_settings->throw_if_table_def_differs) + if (restore_settings->allow_different_table_def) return; getStorageCreateQuery(); @@ -330,7 +336,6 @@ namespace /// Make a create query for this table. auto create_query = renameInCreateQuery(readCreateQueryFromBackup(table_name_)); - create_query->if_not_exists = !restore_settings.throw_if_table_exists; CreateTableInfo info; info.create_query = create_query; @@ -416,8 +421,6 @@ namespace db_name_in_backup.clear(); } - create_db_query->if_not_exists = !restore_settings.throw_if_database_exists; - CreateDatabaseInfo info_db; info_db.create_query = create_db_query; info_db.name_in_backup = std::move(db_name_in_backup); diff --git a/src/Common/ErrorCodes.cpp b/src/Common/ErrorCodes.cpp index c7282939556..e2298e04b44 100644 --- a/src/Common/ErrorCodes.cpp +++ b/src/Common/ErrorCodes.cpp @@ -621,6 +621,7 @@ M(650, SERIALIZATION_ERROR) \ M(651, CAPN_PROTO_BAD_TYPE) \ M(652, ONLY_NULLS_WHILE_READING_SCHEMA) \ + M(653, CANNOT_PARSE_RESTORE_TABLE_CREATION_MODE) \ \ M(999, KEEPER_EXCEPTION) \ M(1000, POCO_EXCEPTION) \ diff --git a/tests/integration/test_backup_restore_new/test.py b/tests/integration/test_backup_restore_new/test.py index 32ad0fbebbc..2bc72c30bdc 100644 --- a/tests/integration/test_backup_restore_new/test.py +++ b/tests/integration/test_backup_restore_new/test.py @@ -78,12 +78,12 @@ def test_restore_table_into_existing_table(engine): instance.query(f"BACKUP TABLE test.table TO {backup_name}") instance.query( - f"RESTORE TABLE test.table INTO test.table FROM {backup_name} SETTINGS throw_if_table_exists=0" + f"RESTORE TABLE test.table INTO test.table FROM {backup_name}" ) assert instance.query("SELECT count(), sum(x) FROM test.table") == "200\t9900\n" instance.query( - f"RESTORE TABLE test.table INTO test.table FROM {backup_name} SETTINGS throw_if_table_exists=0" + f"RESTORE TABLE test.table INTO test.table FROM {backup_name}" ) assert instance.query("SELECT count(), sum(x) FROM test.table") == "300\t14850\n"