From 80e082d3e26e242b57c92e0564e1b5d0e39c4f6a Mon Sep 17 00:00:00 2001 From: Alexander Tokmakov Date: Thu, 28 Nov 2019 22:40:51 +0300 Subject: [PATCH] try fix backward compatibility --- dbms/programs/server/Server.cpp | 6 +---- dbms/src/Databases/DatabaseAtomic.cpp | 1 + dbms/src/Databases/DatabaseLazy.cpp | 3 +-- dbms/src/Databases/DatabaseOnDisk.cpp | 10 +++++++ dbms/src/Databases/DatabaseOnDisk.h | 5 +--- dbms/src/Databases/DatabaseOrdinary.cpp | 3 +-- dbms/src/Databases/DatabaseWithDictionaries.h | 4 +-- .../Interpreters/InterpreterCreateQuery.cpp | 10 +++---- dbms/src/Interpreters/loadMetadata.cpp | 27 ++++++++++--------- dbms/src/Interpreters/loadMetadata.h | 2 +- docker/test/stateful/Dockerfile | 2 +- docker/test/stress/Dockerfile | 2 +- 12 files changed, 39 insertions(+), 36 deletions(-) diff --git a/dbms/programs/server/Server.cpp b/dbms/programs/server/Server.cpp index bd820e5eb0a..7a30f43f928 100644 --- a/dbms/programs/server/Server.cpp +++ b/dbms/programs/server/Server.cpp @@ -268,10 +268,6 @@ int Server::main(const std::vector & /*args*/) global_context->setPath(path); - /// Create directories for 'path' and for default database, if not exist. - Poco::File(path + "data/" + default_database).createDirectories(); - Poco::File(path + "metadata/" + default_database).createDirectories(); - /// Check that we have read and write access to all data paths auto disk_selector = global_context->getDiskSelector(); for (const auto & [name, disk] : disk_selector.getDisksMap()) @@ -529,7 +525,7 @@ int Server::main(const std::vector & /*args*/) /// After the system database is created, attach virtual system tables (in addition to query_log and part_log) attachSystemTablesServer(*global_context->getDatabase("system"), has_zookeeper); /// Then, load remaining databases - loadMetadata(*global_context); + loadMetadata(*global_context, default_database); } catch (...) { diff --git a/dbms/src/Databases/DatabaseAtomic.cpp b/dbms/src/Databases/DatabaseAtomic.cpp index 8fdffd54623..2e317bbb0af 100644 --- a/dbms/src/Databases/DatabaseAtomic.cpp +++ b/dbms/src/Databases/DatabaseAtomic.cpp @@ -17,6 +17,7 @@ DatabaseAtomic::DatabaseAtomic(String name_, String metadata_path_, const Contex : DatabaseOrdinary(name_, metadata_path_, context_) { data_path = "store/"; + log = &Logger::get("DatabaseAtomic (" + name_ + ")"); } String DatabaseAtomic::getDataPath(const String & table_name) const diff --git a/dbms/src/Databases/DatabaseLazy.cpp b/dbms/src/Databases/DatabaseLazy.cpp index fb3efaf95e4..f853e36761a 100644 --- a/dbms/src/Databases/DatabaseLazy.cpp +++ b/dbms/src/Databases/DatabaseLazy.cpp @@ -30,10 +30,9 @@ namespace ErrorCodes DatabaseLazy::DatabaseLazy(const String & name_, const String & metadata_path_, time_t expiration_time_, const Context & context_) - : DatabaseOnDisk(name_, metadata_path_, "DatabaseLazy (" + name_ + ")") + : DatabaseOnDisk(name_, metadata_path_, "DatabaseLazy (" + name_ + ")", context_) , expiration_time(expiration_time_) { - Poco::File(context_.getPath() + getDataPath()).createDirectories(); } diff --git a/dbms/src/Databases/DatabaseOnDisk.cpp b/dbms/src/Databases/DatabaseOnDisk.cpp index d5a734c0d98..9257ab17c83 100644 --- a/dbms/src/Databases/DatabaseOnDisk.cpp +++ b/dbms/src/Databases/DatabaseOnDisk.cpp @@ -142,6 +142,16 @@ String getObjectDefinitionFromCreateQuery(const ASTPtr & query) return statement_stream.str(); } +DatabaseOnDisk::DatabaseOnDisk(const String & name, const String & metadata_path_, const String & logger, const Context & context_) + : DatabaseWithOwnTablesBase(name, logger) + , metadata_path(metadata_path_) + , data_path("data/" + escapeForFileName(database_name) + "/") +{ + Poco::File(context_.getPath() + getDataPath()).createDirectories(); + Poco::File(getMetadataPath()).createDirectories(); +} + + void DatabaseOnDisk::createTable( const Context & context, const String & table_name, diff --git a/dbms/src/Databases/DatabaseOnDisk.h b/dbms/src/Databases/DatabaseOnDisk.h index 36c33b1042e..fa522ee8ef3 100644 --- a/dbms/src/Databases/DatabaseOnDisk.h +++ b/dbms/src/Databases/DatabaseOnDisk.h @@ -32,10 +32,7 @@ String getObjectDefinitionFromCreateQuery(const ASTPtr & query); class DatabaseOnDisk : public DatabaseWithOwnTablesBase { public: - DatabaseOnDisk(const String & name, const String & metadata_path_, const String & logger) - : DatabaseWithOwnTablesBase(name, logger) - , metadata_path(metadata_path_) - , data_path("data/" + escapeForFileName(database_name) + "/") {} + DatabaseOnDisk(const String & name, const String & metadata_path_, const String & logger, const Context & context_); void createTable( const Context & context, diff --git a/dbms/src/Databases/DatabaseOrdinary.cpp b/dbms/src/Databases/DatabaseOrdinary.cpp index 62bde28dd17..fa17376013b 100644 --- a/dbms/src/Databases/DatabaseOrdinary.cpp +++ b/dbms/src/Databases/DatabaseOrdinary.cpp @@ -93,9 +93,8 @@ void logAboutProgress(Poco::Logger * log, size_t processed, size_t total, Atomic DatabaseOrdinary::DatabaseOrdinary(const String & name_, const String & metadata_path_, const Context & context_) - : DatabaseWithDictionaries(name_, metadata_path_,"DatabaseOrdinary (" + name_ + ")") + : DatabaseWithDictionaries(name_, metadata_path_,"DatabaseOrdinary (" + name_ + ")", context_) { - Poco::File(context_.getPath() + getDataPath()).createDirectories(); } diff --git a/dbms/src/Databases/DatabaseWithDictionaries.h b/dbms/src/Databases/DatabaseWithDictionaries.h index 59e8023b730..bf900418c23 100644 --- a/dbms/src/Databases/DatabaseWithDictionaries.h +++ b/dbms/src/Databases/DatabaseWithDictionaries.h @@ -26,8 +26,8 @@ public: bool isDictionaryExist(const Context & context, const String & dictionary_name) const override; protected: - DatabaseWithDictionaries(const String & name, const String & metadata_path_, const String & logger) - : DatabaseOnDisk(name, metadata_path_, logger) {} + DatabaseWithDictionaries(const String & name, const String & metadata_path_, const String & logger, const Context & context_) + : DatabaseOnDisk(name, metadata_path_, logger, context_) {} StoragePtr getDictionaryStorage(const Context & context, const String & table_name) const; diff --git a/dbms/src/Interpreters/InterpreterCreateQuery.cpp b/dbms/src/Interpreters/InterpreterCreateQuery.cpp index 49fecc37c21..74c85c7fce9 100644 --- a/dbms/src/Interpreters/InterpreterCreateQuery.cpp +++ b/dbms/src/Interpreters/InterpreterCreateQuery.cpp @@ -101,9 +101,10 @@ BlockIO InterpreterCreateQuery::createDatabase(ASTCreateQuery & create) { /// For new-style databases engine is explicitly specified in .sql /// When attaching old-style database during server startup, we must always use Ordinary engine - // FIXME maybe throw an exception if it's an ATTACH DATABASE query from user (it's not server startup) and engine is not specified - bool old_style_database = create.attach || - context.getSettingsRef().default_database_engine.value == DefaultDatabaseEngine::Ordinary; + //FIXME is it possible, that database engine is not specified in metadata file? + if (create.attach) + throw Exception("Database engine must be specified for ATTACH DATABASE query", ErrorCodes::UNKNOWN_DATABASE_ENGINE); + bool old_style_database = context.getSettingsRef().default_database_engine.value == DefaultDatabaseEngine::Ordinary; auto engine = std::make_shared(); auto storage = std::make_shared(); engine->name = old_style_database ? "Ordinary" : "Atomic"; @@ -126,11 +127,8 @@ BlockIO InterpreterCreateQuery::createDatabase(ASTCreateQuery & create) String database_name_escaped = escapeForFileName(database_name); - - /// Create directories for tables metadata. String path = context.getPath(); String metadata_path = path + "metadata/" + database_name_escaped + "/"; - Poco::File(metadata_path).createDirectory(); DatabasePtr database = DatabaseFactory::get(database_name, metadata_path, create.storage, context); diff --git a/dbms/src/Interpreters/loadMetadata.cpp b/dbms/src/Interpreters/loadMetadata.cpp index 2a1563b7819..77c2eb7e08f 100644 --- a/dbms/src/Interpreters/loadMetadata.cpp +++ b/dbms/src/Interpreters/loadMetadata.cpp @@ -1,11 +1,6 @@ -#include -#include -#include - #include #include -#include #include #include @@ -21,7 +16,6 @@ #include #include -#include #include @@ -39,7 +33,6 @@ static void executeCreateQuery( ASTPtr ast = parseQuery(parser, query.data(), query.data() + query.size(), "in file " + file_name, 0); auto & ast_create_query = ast->as(); - ast_create_query.attach = true; ast_create_query.database = database; InterpreterCreateQuery interpreter(ast, context); @@ -55,20 +48,27 @@ static void loadDatabase( const String & database_path, bool force_restore_data) { - /// There may exist .sql file with database creation statement. - /// Or, if it is absent, then database with Ordinary engine is created. String database_attach_query; String database_metadata_file = database_path + ".sql"; if (Poco::File(database_metadata_file).exists()) { + /// There are .sql file with database creation statement. ReadBufferFromFile in(database_metadata_file, 1024); readStringUntilEOF(database_attach_query, in); } + else if (Poco::File(database_path).exists()) + { + /// Database exists, but .sql file is absent. It's old-style Ordinary database (e.g. system or default) + database_attach_query = "ATTACH DATABASE " + backQuoteIfNeed(database) + " ENGINE = Ordinary"; + } else - //FIXME - database_attach_query = "ATTACH DATABASE " + backQuoteIfNeed(database) + " ENGINE = Atomic"; + { + /// It's first server run and we need create default and system databases. + /// .sql file with database engine will be written for CREATE query. + database_attach_query = "CREATE DATABASE " + backQuoteIfNeed(database); + } executeCreateQuery(database_attach_query, context, database, database_metadata_file, force_restore_data); @@ -78,7 +78,7 @@ static void loadDatabase( #define SYSTEM_DATABASE "system" -void loadMetadata(Context & context) +void loadMetadata(Context & context, const String & default_database_name) { String path = context.getPath() + "metadata"; @@ -108,6 +108,9 @@ void loadMetadata(Context & context) databases.emplace(unescapeForFileName(it.name()), it.path().toString()); } + if (!default_database_name.empty() && !databases.count(default_database_name)) + databases.emplace(default_database_name, path + "/metadata/" + escapeForFileName(default_database_name)); + for (const auto & [name, db_path] : databases) loadDatabase(context, name, db_path, has_force_restore_data_flag); diff --git a/dbms/src/Interpreters/loadMetadata.h b/dbms/src/Interpreters/loadMetadata.h index b7c78a169f5..b23887d5282 100644 --- a/dbms/src/Interpreters/loadMetadata.h +++ b/dbms/src/Interpreters/loadMetadata.h @@ -11,6 +11,6 @@ class Context; void loadMetadataSystem(Context & context); /// Load tables from databases and add them to context. Database 'system' is ignored. Use separate function to load system tables. -void loadMetadata(Context & context); +void loadMetadata(Context & context, const String & default_database_name = {}); } diff --git a/docker/test/stateful/Dockerfile b/docker/test/stateful/Dockerfile index 1a16cc2f93a..59e3c037265 100644 --- a/docker/test/stateful/Dockerfile +++ b/docker/test/stateful/Dockerfile @@ -39,7 +39,7 @@ CMD dpkg -i package_folder/clickhouse-common-static_*.deb; \ && /s3downloader --dataset-names $DATASETS \ && chmod 777 -R /var/lib/clickhouse \ && clickhouse-client --query "SHOW DATABASES" \ - && clickhouse-client --query "CREATE DATABASE datasets ENGINE = Ordinary" \ + && clickhouse-client --query "ATTACH DATABASE datasets ENGINE = Ordinary" \ && clickhouse-client --query "CREATE DATABASE test" \ && service clickhouse-server restart && sleep 5 \ && clickhouse-client --query "SHOW TABLES FROM datasets" \ diff --git a/docker/test/stress/Dockerfile b/docker/test/stress/Dockerfile index 2d607ee6cd9..116f4ec03f2 100644 --- a/docker/test/stress/Dockerfile +++ b/docker/test/stress/Dockerfile @@ -39,7 +39,7 @@ CMD dpkg -i package_folder/clickhouse-common-static_*.deb; \ service clickhouse-server start && sleep 5 \ && /s3downloader --dataset-names $DATASETS \ && chmod 777 -R /var/lib/clickhouse \ - && clickhouse-client --query "CREATE DATABASE IF NOT EXISTS datasets ENGINE = Ordinary" \ + && clickhouse-client --query "ATTACH DATABASE IF NOT EXISTS datasets ENGINE = Ordinary" \ && clickhouse-client --query "CREATE DATABASE IF NOT EXISTS test" \ && service clickhouse-server restart && sleep 5 \ && clickhouse-client --query "SHOW TABLES FROM datasets" \