From 03ccf05d14bcd6bcdd5346a0aa73ee3fd96f743a Mon Sep 17 00:00:00 2001 From: NikBarykin Date: Tue, 3 Sep 2024 16:10:46 +0300 Subject: [PATCH 1/4] Allow custom settings in database engine --- src/Databases/DatabaseFactory.cpp | 21 +++++++---------- src/Databases/DatabaseFactory.h | 23 ++++++++++++++++--- src/Databases/DatabaseFilesystem.cpp | 2 +- src/Databases/DatabaseHDFS.cpp | 2 +- src/Databases/DatabaseLazy.cpp | 2 +- src/Databases/DatabaseReplicated.cpp | 2 +- src/Databases/DatabaseS3.cpp | 2 +- .../MySQL/DatabaseMaterializedMySQL.cpp | 10 ++++++-- src/Databases/MySQL/DatabaseMySQL.cpp | 2 +- .../DatabaseMaterializedPostgreSQL.cpp | 6 ++++- .../PostgreSQL/DatabasePostgreSQL.cpp | 2 +- src/Databases/SQLite/DatabaseSQLite.cpp | 2 +- 12 files changed, 49 insertions(+), 27 deletions(-) diff --git a/src/Databases/DatabaseFactory.cpp b/src/Databases/DatabaseFactory.cpp index 05a5e057c55..358cdccf8c5 100644 --- a/src/Databases/DatabaseFactory.cpp +++ b/src/Databases/DatabaseFactory.cpp @@ -66,28 +66,23 @@ void validate(const ASTCreateQuery & create_query) { auto * storage = create_query.storage; - /// Check engine may have arguments - static const std::unordered_set engines_with_arguments{"MySQL", "MaterializeMySQL", "MaterializedMySQL", - "Lazy", "Replicated", "PostgreSQL", "MaterializedPostgreSQL", "SQLite", "Filesystem", "S3", "HDFS"}; - const String & engine_name = storage->engine->name; - bool engine_may_have_arguments = engines_with_arguments.contains(engine_name); + const EngineFeatures & engine_features = database_engines.at(engine_name).features; - if (storage->engine->arguments && !engine_may_have_arguments) + /// Check engine may have arguments + if (storage->engine->arguments && !engine_features.supports_arguments) throw Exception(ErrorCodes::BAD_ARGUMENTS, "Database engine `{}` cannot have arguments", engine_name); /// Check engine may have settings - bool may_have_settings = endsWith(engine_name, "MySQL") || engine_name == "Replicated" || engine_name == "MaterializedPostgreSQL"; bool has_unexpected_element = storage->engine->parameters || storage->partition_by || storage->primary_key || storage->order_by || storage->sample_by; - if (has_unexpected_element || (!may_have_settings && storage->settings)) + if (has_unexpected_element || (!engine_features.supports_settings && storage->settings)) throw Exception(ErrorCodes::UNKNOWN_ELEMENT_IN_AST, "Database engine `{}` cannot have parameters, primary_key, order_by, sample_by, settings", engine_name); /// Check engine with table overrides - static const std::unordered_set engines_with_table_overrides{"MaterializeMySQL", "MaterializedMySQL", "MaterializedPostgreSQL"}; - if (create_query.table_overrides && !engines_with_table_overrides.contains(engine_name)) + if (create_query.table_overrides && !engine_features.supports_table_overrides) throw Exception(ErrorCodes::BAD_ARGUMENTS, "Database engine `{}` cannot have table overrides", engine_name); } @@ -121,9 +116,9 @@ DatabasePtr DatabaseFactory::get(const ASTCreateQuery & create, const String & m return impl; } -void DatabaseFactory::registerDatabase(const std::string & name, CreatorFn creator_fn) +void DatabaseFactory::registerDatabase(const std::string & name, CreatorFn creator_fn, EngineFeatures features) { - if (!database_engines.emplace(name, std::move(creator_fn)).second) + if (!database_engines.emplace(name, Creator{std::move(creator_fn), features}).second) throw Exception(ErrorCodes::LOGICAL_ERROR, "DatabaseFactory: the database engine name '{}' is not unique", name); } @@ -154,7 +149,7 @@ DatabasePtr DatabaseFactory::getImpl(const ASTCreateQuery & create, const String .context = context}; // creator_fn creates and returns a DatabasePtr with the supplied arguments - auto creator_fn = database_engines.at(engine_name); + auto creator_fn = database_engines.at(engine_name).creator_fn; return creator_fn(arguments); } diff --git a/src/Databases/DatabaseFactory.h b/src/Databases/DatabaseFactory.h index 494c9e0076e..36275820cbe 100644 --- a/src/Databases/DatabaseFactory.h +++ b/src/Databases/DatabaseFactory.h @@ -43,13 +43,30 @@ public: ContextPtr & context; }; - DatabasePtr get(const ASTCreateQuery & create, const String & metadata_path, ContextPtr context); + struct EngineFeatures + { + bool supports_arguments = false; + bool supports_settings = false; + bool supports_table_overrides = false; + }; using CreatorFn = std::function; - using DatabaseEngines = std::unordered_map; + struct Creator + { + CreatorFn creator_fn; + EngineFeatures features; + }; - void registerDatabase(const std::string & name, CreatorFn creator_fn); + DatabasePtr get(const ASTCreateQuery & create, const String & metadata_path, ContextPtr context); + + using DatabaseEngines = std::unordered_map; + + void registerDatabase(const std::string & name, CreatorFn creator_fn, EngineFeatures features = EngineFeatures{ + supports_arguments = false, + supports_parameters = false, + supports_table_overrides = false, + }); const DatabaseEngines & getDatabaseEngines() const { return database_engines; } diff --git a/src/Databases/DatabaseFilesystem.cpp b/src/Databases/DatabaseFilesystem.cpp index 31701e665a1..4b50e79da4a 100644 --- a/src/Databases/DatabaseFilesystem.cpp +++ b/src/Databases/DatabaseFilesystem.cpp @@ -257,6 +257,6 @@ void registerDatabaseFilesystem(DatabaseFactory & factory) return std::make_shared(args.database_name, init_path, args.context); }; - factory.registerDatabase("Filesystem", create_fn); + factory.registerDatabase("Filesystem", create_fn, {.supports_arguments = true}); } } diff --git a/src/Databases/DatabaseHDFS.cpp b/src/Databases/DatabaseHDFS.cpp index 7fa67a5678e..ceca2666e49 100644 --- a/src/Databases/DatabaseHDFS.cpp +++ b/src/Databases/DatabaseHDFS.cpp @@ -253,7 +253,7 @@ void registerDatabaseHDFS(DatabaseFactory & factory) return std::make_shared(args.database_name, source_url, args.context); }; - factory.registerDatabase("HDFS", create_fn); + factory.registerDatabase("HDFS", create_fn, {.supports_arguments = true}); } } // DB diff --git a/src/Databases/DatabaseLazy.cpp b/src/Databases/DatabaseLazy.cpp index 2ccdd8510a8..0a4b02c4917 100644 --- a/src/Databases/DatabaseLazy.cpp +++ b/src/Databases/DatabaseLazy.cpp @@ -398,6 +398,6 @@ void registerDatabaseLazy(DatabaseFactory & factory) cache_expiration_time_seconds, args.context); }; - factory.registerDatabase("Lazy", create_fn); + factory.registerDatabase("Lazy", create_fn, {.supports_arguments = true}); } } diff --git a/src/Databases/DatabaseReplicated.cpp b/src/Databases/DatabaseReplicated.cpp index 8e3378bcc12..53064703c2c 100644 --- a/src/Databases/DatabaseReplicated.cpp +++ b/src/Databases/DatabaseReplicated.cpp @@ -1949,6 +1949,6 @@ void registerDatabaseReplicated(DatabaseFactory & factory) replica_name, std::move(database_replicated_settings), args.context); }; - factory.registerDatabase("Replicated", create_fn); + factory.registerDatabase("Replicated", create_fn, {.supports_arguments = true, .supports_settings = true}); } } diff --git a/src/Databases/DatabaseS3.cpp b/src/Databases/DatabaseS3.cpp index 2b2d978a846..d80cc6d0953 100644 --- a/src/Databases/DatabaseS3.cpp +++ b/src/Databases/DatabaseS3.cpp @@ -326,7 +326,7 @@ void registerDatabaseS3(DatabaseFactory & factory) return std::make_shared(args.database_name, config, args.context); }; - factory.registerDatabase("S3", create_fn); + factory.registerDatabase("S3", create_fn, {.supports_arguments = true}); } } #endif diff --git a/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp b/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp index 2f5477a6b9d..50c7a5bf588 100644 --- a/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp +++ b/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp @@ -290,8 +290,14 @@ void registerDatabaseMaterializedMySQL(DatabaseFactory & factory) binlog_client, std::move(materialize_mode_settings)); }; - factory.registerDatabase("MaterializeMySQL", create_fn); - factory.registerDatabase("MaterializedMySQL", create_fn); + + DatabaseFactory::Features features{ + .supports_arguments = true, + .supports_settings = true, + .supports_table_overrides = true, + } + factory.registerDatabase("MaterializeMySQL", create_fn, features); + factory.registerDatabase("MaterializedMySQL", create_fn, features); } } diff --git a/src/Databases/MySQL/DatabaseMySQL.cpp b/src/Databases/MySQL/DatabaseMySQL.cpp index 7aa29018f4d..3b72f2aeae5 100644 --- a/src/Databases/MySQL/DatabaseMySQL.cpp +++ b/src/Databases/MySQL/DatabaseMySQL.cpp @@ -584,7 +584,7 @@ void registerDatabaseMySQL(DatabaseFactory & factory) throw Exception(ErrorCodes::CANNOT_CREATE_DATABASE, "Cannot create MySQL database, because {}", exception_message); } }; - factory.registerDatabase("MySQL", create_fn); + factory.registerDatabase("MySQL", create_fn, {.supports_arguments = true, .supports_settings = true}); } } diff --git a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp index 6b0548b85c7..ed62398e594 100644 --- a/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp +++ b/src/Databases/PostgreSQL/DatabaseMaterializedPostgreSQL.cpp @@ -546,7 +546,11 @@ void registerDatabaseMaterializedPostgreSQL(DatabaseFactory & factory) args.database_name, configuration.database, connection_info, std::move(postgresql_replica_settings)); }; - factory.registerDatabase("MaterializedPostgreSQL", create_fn); + factory.registerDatabase("MaterializedPostgreSQL", create_fn, { + .supports_arguments = true, + .supports_settings = true, + .supports_table_overrides = true, + }); } } diff --git a/src/Databases/PostgreSQL/DatabasePostgreSQL.cpp b/src/Databases/PostgreSQL/DatabasePostgreSQL.cpp index 032fc33ea16..0eafd1c3b5b 100644 --- a/src/Databases/PostgreSQL/DatabasePostgreSQL.cpp +++ b/src/Databases/PostgreSQL/DatabasePostgreSQL.cpp @@ -558,7 +558,7 @@ void registerDatabasePostgreSQL(DatabaseFactory & factory) pool, use_table_cache); }; - factory.registerDatabase("PostgreSQL", create_fn); + factory.registerDatabase("PostgreSQL", create_fn, {.supports_arguments = true}); } } diff --git a/src/Databases/SQLite/DatabaseSQLite.cpp b/src/Databases/SQLite/DatabaseSQLite.cpp index 471730fce29..5af9eb1920e 100644 --- a/src/Databases/SQLite/DatabaseSQLite.cpp +++ b/src/Databases/SQLite/DatabaseSQLite.cpp @@ -220,7 +220,7 @@ void registerDatabaseSQLite(DatabaseFactory & factory) return std::make_shared(args.context, engine_define, args.create_query.attach, database_path); }; - factory.registerDatabase("SQLite", create_fn); + factory.registerDatabase("SQLite", create_fn, {.supports_arguments = true}); } } From e874c6e1de7a8b1bc92fd1e60dcdbc53caac3ded Mon Sep 17 00:00:00 2001 From: NikBarykin Date: Tue, 3 Sep 2024 18:58:39 +0300 Subject: [PATCH 2/4] Fix typo --- src/Databases/DatabaseFactory.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Databases/DatabaseFactory.h b/src/Databases/DatabaseFactory.h index 36275820cbe..09ebd2bb2b2 100644 --- a/src/Databases/DatabaseFactory.h +++ b/src/Databases/DatabaseFactory.h @@ -64,7 +64,7 @@ public: void registerDatabase(const std::string & name, CreatorFn creator_fn, EngineFeatures features = EngineFeatures{ supports_arguments = false, - supports_parameters = false, + supports_settings = false, supports_table_overrides = false, }); From 83854cf293bf52376aa9aa5359fa606f4ccd713e Mon Sep 17 00:00:00 2001 From: NikBarykin Date: Tue, 3 Sep 2024 19:13:05 +0300 Subject: [PATCH 3/4] Make method of DatabaseFactory --- src/Databases/DatabaseFactory.cpp | 5 +---- src/Databases/DatabaseFactory.h | 10 +++++++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Databases/DatabaseFactory.cpp b/src/Databases/DatabaseFactory.cpp index 358cdccf8c5..d97474bd245 100644 --- a/src/Databases/DatabaseFactory.cpp +++ b/src/Databases/DatabaseFactory.cpp @@ -59,10 +59,7 @@ void cckMetadataPathForOrdinary(const ASTCreateQuery & create, const String & me } -/// validate validates the database engine that's specified in the create query for -/// engine arguments, settings and table overrides. -void validate(const ASTCreateQuery & create_query) - +void DatabaseFactory::validate(const ASTCreateQuery & create_query) const { auto * storage = create_query.storage; diff --git a/src/Databases/DatabaseFactory.h b/src/Databases/DatabaseFactory.h index 09ebd2bb2b2..ede4394e435 100644 --- a/src/Databases/DatabaseFactory.h +++ b/src/Databases/DatabaseFactory.h @@ -63,9 +63,9 @@ public: using DatabaseEngines = std::unordered_map; void registerDatabase(const std::string & name, CreatorFn creator_fn, EngineFeatures features = EngineFeatures{ - supports_arguments = false, - supports_settings = false, - supports_table_overrides = false, + .supports_arguments = false, + .supports_settings = false, + .supports_table_overrides = false, }); const DatabaseEngines & getDatabaseEngines() const { return database_engines; } @@ -82,6 +82,10 @@ private: DatabaseEngines database_engines; DatabasePtr getImpl(const ASTCreateQuery & create, const String & metadata_path, ContextPtr context); + + /// validate validates the database engine that's specified in the create query for + /// engine arguments, settings and table overrides. + void validate(const ASTCreateQuery & create_query) const; }; } From 4b69d8e2ca2a68e2030d31151d6adb63a79de836 Mon Sep 17 00:00:00 2001 From: NikBarykin Date: Tue, 17 Sep 2024 15:52:20 +0300 Subject: [PATCH 4/4] Fix CE --- src/Databases/MySQL/DatabaseMaterializedMySQL.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp b/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp index 50c7a5bf588..2b728039632 100644 --- a/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp +++ b/src/Databases/MySQL/DatabaseMaterializedMySQL.cpp @@ -291,11 +291,11 @@ void registerDatabaseMaterializedMySQL(DatabaseFactory & factory) std::move(materialize_mode_settings)); }; - DatabaseFactory::Features features{ + DatabaseFactory::EngineFeatures features{ .supports_arguments = true, .supports_settings = true, .supports_table_overrides = true, - } + }; factory.registerDatabase("MaterializeMySQL", create_fn, features); factory.registerDatabase("MaterializedMySQL", create_fn, features); }