diff --git a/src/Backups/RestorerFromBackup.cpp b/src/Backups/RestorerFromBackup.cpp index ce2c02cf120..2620ae16168 100644 --- a/src/Backups/RestorerFromBackup.cpp +++ b/src/Backups/RestorerFromBackup.cpp @@ -37,6 +37,8 @@ namespace ErrorCodes namespace { + constexpr const std::string_view sql_ext = ".sql"; + bool hasSystemTableEngine(const IAST & ast) { const ASTCreateQuery * create = ast.as(); @@ -284,7 +286,7 @@ void RestorerFromBackup::collectDatabaseAndTableInfos() } case ASTBackupQuery::ElementType::DATABASE: { - collectDatabaseInfo(element.database_name, element.except_tables); + collectDatabaseInfo(element.database_name, element.except_tables, /* throw_if_no_database_metadata_in_backup= */ true); break; } case ASTBackupQuery::ElementType::ALL: @@ -380,7 +382,7 @@ void RestorerFromBackup::collectTableInfo(const QualifiedTableName & table_name_ } } -void RestorerFromBackup::collectDatabaseInfo(const String & database_name_in_backup, const std::set & except_table_names) +void RestorerFromBackup::collectDatabaseInfo(const String & database_name_in_backup, const std::set & except_table_names, bool throw_if_no_database_metadata_in_backup) { std::optional metadata_path; std::unordered_set table_names_in_backup; @@ -393,7 +395,6 @@ void RestorerFromBackup::collectDatabaseInfo(const String & database_name_in_bac Strings file_names = backup->listFiles(root_path_in_backup / "metadata" / escapeForFileName(database_name_in_backup)); for (const String & file_name : file_names) { - constexpr const std::string_view sql_ext = ".sql"; if (!file_name.ends_with(sql_ext)) continue; String file_name_without_ext = file_name.substr(0, file_name.length() - sql_ext.length()); @@ -401,12 +402,9 @@ void RestorerFromBackup::collectDatabaseInfo(const String & database_name_in_bac } } - if (!metadata_path && table_names_in_backup.empty()) + if (!metadata_path && throw_if_no_database_metadata_in_backup) throw Exception(ErrorCodes::BACKUP_ENTRY_NOT_FOUND, "Database {} not found in backup", backQuoteIfNeed(database_name_in_backup)); - String database_name = renaming_map.getNewDatabaseName(database_name_in_backup); - - ASTPtr create_database_query; if (metadata_path) { auto read_buffer = backup->readFile(*metadata_path)->getReadBuffer(); @@ -414,52 +412,57 @@ void RestorerFromBackup::collectDatabaseInfo(const String & database_name_in_bac readStringUntilEOF(create_query_str, *read_buffer); read_buffer.reset(); ParserCreateQuery create_parser; - create_database_query = parseQuery(create_parser, create_query_str, 0, DBMS_DEFAULT_MAX_PARSER_DEPTH); + ASTPtr create_database_query = parseQuery(create_parser, create_query_str, 0, DBMS_DEFAULT_MAX_PARSER_DEPTH); renameDatabaseAndTableNameInCreateQuery(context->getGlobalContext(), renaming_map, create_database_query); - } - else - { - auto generated_create_query = std::make_shared(); - generated_create_query->setDatabase(database_name); - create_database_query = generated_create_query; - } - DatabaseInfo & database_info = database_infos[database_name]; - - if (database_info.create_database_query && (serializeAST(*database_info.create_database_query) != serializeAST(*create_database_query))) - { - throw Exception( - ErrorCodes::CANNOT_RESTORE_DATABASE, - "Extracted two different create queries for the same database {}: {} and {}", - backQuoteIfNeed(database_name), - serializeAST(*database_info.create_database_query), - serializeAST(*create_database_query)); - } + String database_name = renaming_map.getNewDatabaseName(database_name_in_backup); + DatabaseInfo & database_info = database_infos[database_name]; + + if (database_info.create_database_query && (serializeAST(*database_info.create_database_query) != serializeAST(*create_database_query))) + { + throw Exception( + ErrorCodes::CANNOT_RESTORE_DATABASE, + "Extracted two different create queries for the same database {}: {} and {}", + backQuoteIfNeed(database_name), + serializeAST(*database_info.create_database_query), + serializeAST(*create_database_query)); + } - database_info.create_database_query = create_database_query; + database_info.create_database_query = create_database_query; + } for (const String & table_name_in_backup : table_names_in_backup) { if (except_table_names.contains({database_name_in_backup, table_name_in_backup})) continue; - collectTableInfo({database_name_in_backup, table_name_in_backup}, /* is_temporary_table= */ false, {}); + collectTableInfo({database_name_in_backup, table_name_in_backup}, /* is_temporary_table= */ false, /* partitions= */ {}); } } void RestorerFromBackup::collectAllDatabasesInfo(const std::set & except_database_names, const std::set & except_table_names) { std::unordered_set database_names_in_backup; + std::unordered_set temporary_table_names_in_backup; + for (const auto & root_path_in_backup : root_paths_in_backup) { Strings file_names = backup->listFiles(root_path_in_backup / "metadata"); for (String & file_name : file_names) { - constexpr const std::string_view sql_ext = ".sql"; if (file_name.ends_with(sql_ext)) file_name.resize(file_name.length() - sql_ext.length()); database_names_in_backup.emplace(unescapeForFileName(file_name)); } + + file_names = backup->listFiles(root_path_in_backup / "temporary_tables" / "metadata"); + for (String & file_name : file_names) + { + if (!file_name.ends_with(sql_ext)) + continue; + file_name.resize(file_name.length() - sql_ext.length()); + temporary_table_names_in_backup.emplace(unescapeForFileName(file_name)); + } } for (const String & database_name_in_backup : database_names_in_backup) @@ -467,8 +470,11 @@ void RestorerFromBackup::collectAllDatabasesInfo(const std::set & except if (except_database_names.contains(database_name_in_backup)) continue; - collectDatabaseInfo(database_name_in_backup, except_table_names); + collectDatabaseInfo(database_name_in_backup, except_table_names, /* throw_if_no_database_metadata_in_backup= */ false); } + + for (const String & temporary_table_name_in_backup : temporary_table_names_in_backup) + collectTableInfo({"", temporary_table_name_in_backup}, /* is_temporary_table= */ true, /* partitions= */ {}); } void RestorerFromBackup::createDatabases() diff --git a/src/Backups/RestorerFromBackup.h b/src/Backups/RestorerFromBackup.h index 775a1cf11b8..f99f6d52fff 100644 --- a/src/Backups/RestorerFromBackup.h +++ b/src/Backups/RestorerFromBackup.h @@ -100,7 +100,7 @@ private: void findRootPathsInBackup(); void collectDatabaseAndTableInfos(); void collectTableInfo(const QualifiedTableName & table_name_in_backup, bool is_temporary, const std::optional & partitions); - void collectDatabaseInfo(const String & database_name_in_backup, const std::set & except_table_names); + void collectDatabaseInfo(const String & database_name_in_backup, const std::set & except_table_names, bool throw_if_no_database_metadata_in_backup); void collectAllDatabasesInfo(const std::set & except_database_names, const std::set & except_table_names); void createDatabases(); void createTables(); diff --git a/tests/integration/helpers/cluster.py b/tests/integration/helpers/cluster.py index f8ad9213e5b..53acdfec0d8 100644 --- a/tests/integration/helpers/cluster.py +++ b/tests/integration/helpers/cluster.py @@ -2984,6 +2984,7 @@ class ClickHouseInstance: def http_query( self, sql, + method=None, data=None, params=None, user=None, @@ -3016,10 +3017,11 @@ class ClickHouseInstance: requester = requests.Session() requester.mount("https://", adapter) requester.mount("http://", adapter) - if data: - r = requester.post(url, data, auth=auth, timeout=timeout) - else: - r = requester.get(url, auth=auth, timeout=timeout) + + if method is None: + method = "POST" if data else "GET" + + r = requester.request(method, url, data=data, auth=auth, timeout=timeout) def http_code_and_message(): code = r.status_code diff --git a/tests/integration/test_backup_restore_new/test.py b/tests/integration/test_backup_restore_new/test.py index 0b7d149b97a..7572157d50a 100644 --- a/tests/integration/test_backup_restore_new/test.py +++ b/tests/integration/test_backup_restore_new/test.py @@ -449,7 +449,7 @@ def test_temporary_table(): ) == TSV([["e"], ["q"], ["w"]]) -# We allow BACKUP DATABASE _temporary_and_external_tables only if the backup doesn't contain any table. +# "BACKUP DATABASE _temporary_and_external_tables" is allowed but the backup must not contain any tables. def test_temporary_tables_database(): session_id = new_session_id() instance.http_query( @@ -464,6 +464,33 @@ def test_temporary_tables_database(): ] +def test_restore_all_restores_temporary_tables(): + session_id = new_session_id() + instance.http_query( + "CREATE TEMPORARY TABLE temp_tbl(s String)", params={"session_id": session_id} + ) + instance.http_query( + "INSERT INTO temp_tbl VALUES ('q'), ('w'), ('e')", params={"session_id": session_id} + ) + + backup_name = new_backup_name() + instance.http_query( + f"BACKUP TEMPORARY TABLE temp_tbl TO {backup_name}", + params={"session_id": session_id}, + ) + + session_id = new_session_id() + instance.http_query( + f"RESTORE ALL FROM {backup_name}", + params={"session_id": session_id}, + method="POST" + ) + + assert instance.http_query( + "SELECT * FROM temp_tbl ORDER BY s", params={"session_id": session_id} + ) == TSV([["e"], ["q"], ["w"]]) + + def test_system_table(): backup_name = new_backup_name() instance.query(f"BACKUP TABLE system.numbers TO {backup_name}")