diff --git a/programs/copier/ClusterCopierApp.cpp b/programs/copier/ClusterCopierApp.cpp index 35c5b2faa87..ce4bf94589e 100644 --- a/programs/copier/ClusterCopierApp.cpp +++ b/programs/copier/ClusterCopierApp.cpp @@ -114,7 +114,7 @@ void ClusterCopierApp::mainImpl() registerDisks(); static const std::string default_database = "_local"; - DatabaseCatalog::instance().attachDatabase(default_database, UUIDHelpers::Nil, std::make_shared(default_database, *context)); + DatabaseCatalog::instance().attachDatabase(default_database, std::make_shared(default_database, *context)); context->setCurrentDatabase(default_database); /// Initialize query scope just in case. diff --git a/programs/local/LocalServer.cpp b/programs/local/LocalServer.cpp index 141c1237bf4..8641287c3ec 100644 --- a/programs/local/LocalServer.cpp +++ b/programs/local/LocalServer.cpp @@ -164,7 +164,7 @@ static void attachSystemTables(const Context & context) { /// TODO: add attachTableDelayed into DatabaseMemory to speedup loading system_database = std::make_shared(DatabaseCatalog::SYSTEM_DATABASE, context); - DatabaseCatalog::instance().attachDatabase(DatabaseCatalog::SYSTEM_DATABASE, UUIDHelpers::Nil, system_database); + DatabaseCatalog::instance().attachDatabase(DatabaseCatalog::SYSTEM_DATABASE, system_database); } attachSystemTablesLocal(*system_database); @@ -241,7 +241,7 @@ try * if such tables will not be dropped, clickhouse-server will not be able to load them due to security reasons. */ std::string default_database = config().getString("default_database", "_local"); - DatabaseCatalog::instance().attachDatabase(default_database, UUIDHelpers::Nil, std::make_shared(default_database, *context)); + DatabaseCatalog::instance().attachDatabase(default_database, std::make_shared(default_database, *context)); context->setCurrentDatabase(default_database); applyCmdOptions(); diff --git a/src/Databases/DatabaseAtomic.cpp b/src/Databases/DatabaseAtomic.cpp index e64c59d0ba0..3f3a9f847dd 100644 --- a/src/Databases/DatabaseAtomic.cpp +++ b/src/Databases/DatabaseAtomic.cpp @@ -389,6 +389,7 @@ void DatabaseAtomic::tryCreateMetadataSymlink() void DatabaseAtomic::renameDatabase(const String & new_name) { + /// CREATE, ATTACH, DROP, DETACH and RENAME DATABASE must hold DDLGuard try { Poco::File(path_to_metadata_symlink).remove(); @@ -398,8 +399,16 @@ void DatabaseAtomic::renameDatabase(const String & new_name) LOG_WARNING(log, getCurrentExceptionMessage(true)); } + auto new_name_escaped = escapeForFileName(new_name); + auto old_database_metadata_path = global_context.getPath() + "metadata/" + escapeForFileName(getDatabaseName()) + ".sql"; + auto new_database_metadata_path = global_context.getPath() + "metadata/" + new_name_escaped + ".sql"; + renameNoReplace(old_database_metadata_path, new_database_metadata_path); + + String old_path_to_table_symlinks; + { std::lock_guard lock(mutex); + DatabaseCatalog::instance().updateDatabaseName(database_name, new_name); database_name = new_name; for (auto & table : tables) @@ -409,12 +418,12 @@ void DatabaseAtomic::renameDatabase(const String & new_name) table.second->renameInMemory(table_id); } - path_to_metadata_symlink = global_context.getPath() + "metadata/" + escapeForFileName(database_name); - String old_path_to_table_symlinks = path_to_table_symlinks; - path_to_table_symlinks = global_context.getPath() + "data/" + escapeForFileName(database_name) + "/"; - Poco::File(old_path_to_table_symlinks).renameTo(path_to_table_symlinks); + path_to_metadata_symlink = global_context.getPath() + "metadata/" + new_name_escaped; + old_path_to_table_symlinks = path_to_table_symlinks; + path_to_table_symlinks = global_context.getPath() + "data/" + new_name_escaped + "/"; } + Poco::File(old_path_to_table_symlinks).renameTo(path_to_table_symlinks); tryCreateMetadataSymlink(); } diff --git a/src/Interpreters/DatabaseCatalog.cpp b/src/Interpreters/DatabaseCatalog.cpp index fc8eadb7ca7..c02e418c296 100644 --- a/src/Interpreters/DatabaseCatalog.cpp +++ b/src/Interpreters/DatabaseCatalog.cpp @@ -115,7 +115,7 @@ void DatabaseCatalog::loadDatabases() drop_delay_sec = global_context->getConfigRef().getInt("database_atomic_delay_before_drop_table_sec", default_drop_delay_sec); auto db_for_temporary_and_external_tables = std::make_shared(TEMPORARY_DATABASE, *global_context); - attachDatabase(TEMPORARY_DATABASE, UUIDHelpers::Nil, db_for_temporary_and_external_tables); + attachDatabase(TEMPORARY_DATABASE, db_for_temporary_and_external_tables); loadMarkedAsDroppedTables(); auto task_holder = global_context->getSchedulePool().createTask("DatabaseCatalog", [this](){ this->dropTableDataTask(); }); @@ -248,13 +248,15 @@ void DatabaseCatalog::assertDatabaseDoesntExistUnlocked(const String & database_ throw Exception("Database " + backQuoteIfNeed(database_name) + " already exists.", ErrorCodes::DATABASE_ALREADY_EXISTS); } -void DatabaseCatalog::attachDatabase(const String & database_name, const UUID & uuid, const DatabasePtr & database) +void DatabaseCatalog::attachDatabase(const String & database_name, const DatabasePtr & database) { std::lock_guard lock{databases_mutex}; assertDatabaseDoesntExistUnlocked(database_name); databases.emplace(database_name, database); - if (uuid != UUIDHelpers::Nil) - db_uuid_map.emplace(uuid, database); + UUID db_uuid = database->getUUID(); + assert((db_uuid != UUIDHelpers::Nil) ^ (dynamic_cast(database.get()) == nullptr)); + if (db_uuid != UUIDHelpers::Nil) + db_uuid_map.emplace(db_uuid, database); } @@ -263,13 +265,18 @@ DatabasePtr DatabaseCatalog::detachDatabase(const String & database_name, bool d if (database_name == TEMPORARY_DATABASE) throw Exception("Cannot detach database with temporary tables.", ErrorCodes::DATABASE_ACCESS_DENIED); - std::shared_ptr db; + DatabasePtr db; { std::lock_guard lock{databases_mutex}; assertDatabaseExistsUnlocked(database_name); db = databases.find(database_name)->second; + db_uuid_map.erase(db->getUUID()); + databases.erase(database_name); + } - if (check_empty) + if (check_empty) + { + try { if (!db->empty()) throw Exception("New table appeared in database being dropped or detached. Try again.", @@ -278,9 +285,11 @@ DatabasePtr DatabaseCatalog::detachDatabase(const String & database_name, bool d if (!drop && database_atomic) database_atomic->assertCanBeDetached(false); } - - db_uuid_map.erase(db->getUUID()); - databases.erase(database_name); + catch (...) + { + attachDatabase(database_name, db); + throw; + } } db->shutdown(); @@ -300,36 +309,15 @@ DatabasePtr DatabaseCatalog::detachDatabase(const String & database_name, bool d return db; } -void DatabaseCatalog::renameDatabase(const String & old_name, const String & new_name) +void DatabaseCatalog::updateDatabaseName(const String & old_name, const String & new_name) { std::lock_guard lock{databases_mutex}; - assertDatabaseExistsUnlocked(old_name); - assertDatabaseDoesntExistUnlocked(new_name); + assert(databases.find(new_name) == databases.end()); auto it = databases.find(old_name); + assert(it != databases.end()); auto db = it->second; - db->renameDatabase(new_name); - databases.erase(it); databases.emplace(new_name, db); - - auto depend_it = view_dependencies.begin(); - while (depend_it != view_dependencies.end()) - { - if (depend_it->first.database_name == old_name) - { - auto table_id = depend_it->first; - auto dependencies = std::move(depend_it->second); - depend_it = view_dependencies.erase(depend_it); - table_id.database_name = new_name; - view_dependencies.emplace(std::move(table_id), std::move(dependencies)); - } - else - ++depend_it; - } - - auto old_database_metadata_path = global_context->getPath() + "metadata/" + escapeForFileName(old_name) + ".sql"; - auto new_database_metadata_path = global_context->getPath() + "metadata/" + escapeForFileName(new_name) + ".sql"; - renameNoReplace(old_database_metadata_path, new_database_metadata_path); } DatabasePtr DatabaseCatalog::getDatabase(const String & database_name) const diff --git a/src/Interpreters/DatabaseCatalog.h b/src/Interpreters/DatabaseCatalog.h index 9da0980b740..6842ee45058 100644 --- a/src/Interpreters/DatabaseCatalog.h +++ b/src/Interpreters/DatabaseCatalog.h @@ -121,9 +121,9 @@ public: DatabasePtr getDatabaseForTemporaryTables() const; DatabasePtr getSystemDatabase() const; - void attachDatabase(const String & database_name, const UUID & uuid, const DatabasePtr & database); + void attachDatabase(const String & database_name, const DatabasePtr & database); DatabasePtr detachDatabase(const String & database_name, bool drop = false, bool check_empty = true); - void renameDatabase(const String & old_name, const String & new_name); + void updateDatabaseName(const String & old_name, const String & new_name); /// database_name must be not empty DatabasePtr getDatabase(const String & database_name) const; diff --git a/src/Interpreters/InterpreterCreateQuery.cpp b/src/Interpreters/InterpreterCreateQuery.cpp index 759aa16a14a..512fe7c8434 100644 --- a/src/Interpreters/InterpreterCreateQuery.cpp +++ b/src/Interpreters/InterpreterCreateQuery.cpp @@ -197,7 +197,7 @@ BlockIO InterpreterCreateQuery::createDatabase(ASTCreateQuery & create) try { /// TODO Attach db only after it was loaded. Now it's not possible because of view dependencies - DatabaseCatalog::instance().attachDatabase(database_name, create.uuid, database); + DatabaseCatalog::instance().attachDatabase(database_name, database); added = true; if (need_write_metadata) diff --git a/src/Interpreters/InterpreterRenameQuery.cpp b/src/Interpreters/InterpreterRenameQuery.cpp index 2be70668cd9..90901b4cfb1 100644 --- a/src/Interpreters/InterpreterRenameQuery.cpp +++ b/src/Interpreters/InterpreterRenameQuery.cpp @@ -5,7 +5,6 @@ #include #include #include -#include namespace DB @@ -89,9 +88,14 @@ BlockIO InterpreterRenameQuery::executeToDatabase(const ASTRenameQuery &, const assert(descriptions.size() == 1); assert(descriptions.front().from_table_name.empty()); assert(descriptions.front().to_table_name.empty()); + const auto & old_name = descriptions.front().from_database_name; const auto & new_name = descriptions.back().to_database_name; - DatabaseCatalog::instance().renameDatabase(old_name, new_name); + auto & catalog = DatabaseCatalog::instance(); + + auto db = catalog.getDatabase(old_name); + catalog.assertDatabaseDoesntExist(new_name); + db->renameDatabase(new_name); return {}; } diff --git a/src/Interpreters/loadMetadata.cpp b/src/Interpreters/loadMetadata.cpp index 438ae6c860c..ac7bb4487fe 100644 --- a/src/Interpreters/loadMetadata.cpp +++ b/src/Interpreters/loadMetadata.cpp @@ -156,7 +156,7 @@ void loadMetadataSystem(Context & context) Poco::File(global_path + "metadata/" SYSTEM_DATABASE).createDirectories(); auto system_database = std::make_shared(SYSTEM_DATABASE, global_path + "metadata/" SYSTEM_DATABASE "/", context); - DatabaseCatalog::instance().attachDatabase(SYSTEM_DATABASE, UUIDHelpers::Nil, system_database); + DatabaseCatalog::instance().attachDatabase(SYSTEM_DATABASE, system_database); } } diff --git a/src/Interpreters/tests/in_join_subqueries_preprocessor.cpp b/src/Interpreters/tests/in_join_subqueries_preprocessor.cpp index 0694ed2def7..2b53277d02f 100644 --- a/src/Interpreters/tests/in_join_subqueries_preprocessor.cpp +++ b/src/Interpreters/tests/in_join_subqueries_preprocessor.cpp @@ -1167,7 +1167,7 @@ TestResult check(const TestEntry & entry) auto storage_distributed_hits = StorageDistributedFake::create("distant_db", "distant_hits", entry.shard_count); DB::DatabasePtr database = std::make_shared("test", "./metadata/test/", context); - DB::DatabaseCatalog::instance().attachDatabase("test", UUIDHelpers::Nil, database); + DB::DatabaseCatalog::instance().attachDatabase("test", database); database->attachTable("visits_all", storage_distributed_visits); database->attachTable("hits_all", storage_distributed_hits); context.setCurrentDatabase("test"); diff --git a/src/Storages/tests/gtest_transform_query_for_external_database.cpp b/src/Storages/tests/gtest_transform_query_for_external_database.cpp index 0c14361902f..318d667d9b0 100644 --- a/src/Storages/tests/gtest_transform_query_for_external_database.cpp +++ b/src/Storages/tests/gtest_transform_query_for_external_database.cpp @@ -42,7 +42,7 @@ private: registerFunctions(); DatabasePtr database = std::make_shared("test", context); database->attachTable("table", StorageMemory::create(StorageID("test", "table"), ColumnsDescription{columns}, ConstraintsDescription{})); - DatabaseCatalog::instance().attachDatabase("test", UUIDHelpers::Nil, database); + DatabaseCatalog::instance().attachDatabase("test", database); context.setCurrentDatabase("test"); } }; diff --git a/tests/queries/0_stateless/01192_rename_database.reference b/tests/queries/0_stateless/01192_rename_database.reference new file mode 100644 index 00000000000..5cccac119f7 --- /dev/null +++ b/tests/queries/0_stateless/01192_rename_database.reference @@ -0,0 +1,22 @@ +ok +CREATE DATABASE test_01192 UUID \'00001192-0000-4000-8000-000000000001\'\nENGINE = Atomic +Atomic store 00001192-0000-4000-8000-000000000001 00001192-0000-4000-8000-000000000001 +ok +ok +renamed +inserted +CREATE DATABASE test_01192_renamed UUID \'00001192-0000-4000-8000-000000000001\'\nENGINE = Atomic +Atomic store 00001192-0000-4000-8000-000000000001 00001192-0000-4000-8000-000000000001 +CREATE DATABASE test_01192_renamed\nENGINE = Atomic +10 45 +inserted +renamed +10 45 +10 45 +ok +ok +renamed +inserted +20 190 +10 45 +10 45 diff --git a/tests/queries/0_stateless/01192_rename_database.sh b/tests/queries/0_stateless/01192_rename_database.sh new file mode 100755 index 00000000000..7390018d4c7 --- /dev/null +++ b/tests/queries/0_stateless/01192_rename_database.sh @@ -0,0 +1,58 @@ +#!/usr/bin/env bash + +CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +. $CURDIR/../shell_config.sh + + +$CLICKHOUSE_CLIENT -q "DROP DATABASE IF EXISTS test_01192" +$CLICKHOUSE_CLIENT -q "DROP DATABASE IF EXISTS test_01192_renamed" +$CLICKHOUSE_CLIENT -q "DROP DATABASE IF EXISTS test_01192_atomic" + +$CLICKHOUSE_CLIENT --default_database_engine=Ordinary -q "CREATE DATABASE test_01192 UUID '00001192-0000-4000-8000-000000000001'" 2>&1| grep -F "does not support" > /dev/null && echo "ok" +$CLICKHOUSE_CLIENT --allow_experimental_database_atomic=1 --default_database_engine=Atomic -q "CREATE DATABASE test_01192 UUID '00001192-0000-4000-8000-000000000001'" + +$CLICKHOUSE_CLIENT --show_table_uuid_in_table_create_query_if_not_nil=1 -q "SHOW CREATE DATABASE test_01192" +$CLICKHOUSE_CLIENT -q "SELECT engine, splitByChar('/', data_path)[-2], uuid, splitByChar('/', metadata_path)[-2] FROM system.databases WHERE name='test_01192'" + +$CLICKHOUSE_CLIENT -q "CREATE TABLE test_01192.mt (n UInt64) ENGINE=MergeTree ORDER BY n" +$CLICKHOUSE_CLIENT -q "INSERT INTO test_01192.mt SELECT number + sleepEachRow(1) FROM numbers(10)" && echo "inserted" & +sleep 1 + +$CLICKHOUSE_CLIENT -q "RENAME DATABASE test_01192 TO default" 2>&1| grep -F "already exists" > /dev/null && echo "ok" +$CLICKHOUSE_CLIENT -q "RENAME DATABASE test_01192_notexisting TO test_01192_renamed" 2>&1| grep -F "doesn't exist" > /dev/null && echo "ok" +$CLICKHOUSE_CLIENT -q "RENAME DATABASE test_01192 TO test_01192_renamed" && echo "renamed" +wait + +$CLICKHOUSE_CLIENT --show_table_uuid_in_table_create_query_if_not_nil=1 -q "SHOW CREATE DATABASE test_01192_renamed" +$CLICKHOUSE_CLIENT -q "SELECT engine, splitByChar('/', data_path)[-2], uuid, splitByChar('/', metadata_path)[-2] FROM system.databases WHERE name='test_01192_renamed'" +$CLICKHOUSE_CLIENT -q "SHOW CREATE DATABASE test_01192_renamed" +$CLICKHOUSE_CLIENT -q "SELECT count(n), sum(n) FROM test_01192_renamed.mt" + +$CLICKHOUSE_CLIENT --default_database_engine=Ordinary -q "CREATE DATABASE test_01192" +$CLICKHOUSE_CLIENT -q "CREATE TABLE test_01192.mt AS test_01192_renamed.mt ENGINE=MergeTree ORDER BY n" +$CLICKHOUSE_CLIENT -q "CREATE TABLE test_01192.rmt AS test_01192_renamed.mt ENGINE=ReplicatedMergeTree('/test/01192/', '1') ORDER BY n" +$CLICKHOUSE_CLIENT -q "CREATE MATERIALIZED VIEW test_01192.mv TO test_01192.rmt AS SELECT * FROM test_01192.mt" + +$CLICKHOUSE_CLIENT -q "INSERT INTO test_01192.mt SELECT number FROM numbers(10)" && echo "inserted" + +$CLICKHOUSE_CLIENT --allow_experimental_database_atomic=1 --default_database_engine=Atomic -q "CREATE DATABASE test_01192_atomic" +$CLICKHOUSE_CLIENT -q "DROP DATABASE test_01192_renamed" +$CLICKHOUSE_CLIENT -q "RENAME TABLE test_01192.mt TO test_01192_atomic.mt, test_01192.rmt TO test_01192_atomic.rmt, test_01192.mv TO test_01192_atomic.mv" && echo "renamed" + +$CLICKHOUSE_CLIENT -q "SELECT count(n), sum(n) FROM test_01192_atomic.mt" +$CLICKHOUSE_CLIENT -q "SELECT count(n), sum(n) FROM test_01192_atomic.rmt" +$CLICKHOUSE_CLIENT -q "SELECT count(n), sum(n) FROM test_01192_atomic.mv" 2>&1| grep -F "doesn't exist" > /dev/null && echo "ok" + +$CLICKHOUSE_CLIENT -q "INSERT INTO test_01192_atomic.mt SELECT number + sleepEachRow(1) + 10 FROM numbers(10)" && echo "inserted" & +sleep 1 + +$CLICKHOUSE_CLIENT -q "RENAME DATABASE test_01192 TO test_01192_renamed" 2>&1| grep -F "not supported" > /dev/null && echo "ok" +$CLICKHOUSE_CLIENT -q "DROP DATABASE test_01192" +$CLICKHOUSE_CLIENT -q "RENAME DATABASE test_01192_atomic TO test_01192" && echo "renamed" +wait + +$CLICKHOUSE_CLIENT -q "SELECT count(n), sum(n) FROM test_01192.mt" +$CLICKHOUSE_CLIENT -q "SELECT count(n), sum(n) FROM test_01192.rmt" +$CLICKHOUSE_CLIENT -q "SELECT count(n), sum(n) FROM test_01192.mv" + +$CLICKHOUSE_CLIENT -q "DROP DATABASE test_01192"