InterpreterCreateQuery code cleanup

dbms/src/Interpreters/InterpreterCreateQuery.cpp

@@ -95,31 +95,22 @@ BlockIO InterpreterCreateQuery::createDatabase(ASTCreateQuery & create)
             throw Exception("Database " + database_name + " already exists.", ErrorCodes::DATABASE_ALREADY_EXISTS);
     }
 
-    String database_engine_name;
     if (!create.storage)
     {
-        database_engine_name = "Ordinary"; /// Default database engine.
         auto engine = std::make_shared<ASTFunction>();
-        engine->name = database_engine_name;
         auto storage = std::make_shared<ASTStorage>();
+        engine->name = "Ordinary";
         storage->set(storage->engine, engine);
         create.set(create.storage, storage);
     }
-    else
+    else if ((create.columns_list && create.columns_list->indices && !create.columns_list->indices->children.empty()))
     {
-        const ASTStorage & storage = *create.storage;
-        const ASTFunction & engine = *storage.engine;
         /// Currently, there are no database engines, that support any arguments.
-        if ((create.columns_list && create.columns_list->indices && !create.columns_list->indices->children.empty()))
-        {
         std::stringstream ostr;
-            formatAST(storage, ostr, false, false);
+        formatAST(*create.storage, ostr, false, false);
         throw Exception("Unknown database engine: " + ostr.str(), ErrorCodes::UNKNOWN_DATABASE_ENGINE);
     }
 
-        database_engine_name = engine.name;
-    }
-
     String database_name_escaped = escapeForFileName(database_name);
 
     /// Create directories for tables metadata.
@@ -153,19 +144,27 @@ BlockIO InterpreterCreateQuery::createDatabase(ASTCreateQuery & create)
         out.close();
     }
 
+    bool added = false;
+    bool renamed = false;
     try
     {
         context.addDatabase(database_name, database);
+        added = true;
 
         if (need_write_metadata)
+        {
             Poco::File(metadata_file_tmp_path).renameTo(metadata_file_path);
+            renamed = true;
+        }
 
         database->loadStoredObjects(context, has_force_restore_data_flag);
     }
     catch (...)
     {
-        if (need_write_metadata)
+        if (renamed)
             Poco::File(metadata_file_tmp_path).remove();
+        if (added)
+            context.detachDatabase(database_name);
 
         throw;
     }
@@ -546,21 +545,19 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create)
         throw Exception("Temporary tables cannot be inside a database. You should not specify a database for a temporary table.",
             ErrorCodes::BAD_DATABASE_FOR_TEMPORARY_TABLE);
 
-    String current_database = context.getCurrentDatabase();
-
-    String database_name = create.database.empty() ? current_database : create.database;
-    String table_name = create.table;
-
     // If this is a stub ATTACH query, read the query definition from the database
     if (create.attach && !create.storage && !create.columns_list)
     {
         // Table SQL definition is available even if the table is detached
-        auto query = context.getCreateTableQuery(database_name, table_name);
+        auto query = context.getCreateTableQuery(create.database, create.table);
         create = query->as<ASTCreateQuery &>(); // Copy the saved create query, but use ATTACH instead of CREATE
         create.attach = true;
     }
 
-    if (create.to_database.empty())
+    String current_database = context.getCurrentDatabase();
+    if (!create.temporary && create.database.empty())
+        create.database = current_database;
+    if (!create.to_table.empty() && create.to_database.empty())
         create.to_database = current_database;
 
     if (create.select && (create.is_view || create.is_materialized_view || create.is_live_view))
@@ -573,16 +570,16 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create)
     TableProperties properties = setProperties(create);
 
     /// Actually creates table
-    bool created = doCreateTable(create, properties, database_name);
+    bool created = doCreateTable(create, properties);
-    if (!created)
+
+    if (!created)   /// Table already exists
         return {};
 
-    return fillTableIfNeeded(create, database_name);
+    return fillTableIfNeeded(create);
 }
 
 bool InterpreterCreateQuery::doCreateTable(const ASTCreateQuery & create,
-                                           const InterpreterCreateQuery::TableProperties & properties,
+                                           const InterpreterCreateQuery::TableProperties & properties)
-                                           const String & database_name)
 {
     std::unique_ptr<DDLGuard> guard;
 
@@ -593,13 +590,13 @@ bool InterpreterCreateQuery::doCreateTable(const ASTCreateQuery & create,
     bool need_add_to_database = !create.temporary || create.is_live_view;
     if (need_add_to_database)
     {
-        database = context.getDatabase(database_name);
+        database = context.getDatabase(create.database);
         data_path = database->getDataPath();
 
         /** If the request specifies IF NOT EXISTS, we allow concurrent CREATE queries (which do nothing).
           * If table doesnt exist, one thread is creating table, while others wait in DDLGuard.
           */
-        guard = context.getDDLGuard(database_name, table_name);
+        guard = context.getDDLGuard(create.database, table_name);
 
         /// Table can be created before or it can be created concurrently in another thread, while we were waiting in DDLGuard.
         if (database->isTableExist(context, table_name))
@@ -611,7 +608,7 @@ bool InterpreterCreateQuery::doCreateTable(const ASTCreateQuery & create,
             {
                 /// when executing CREATE OR REPLACE VIEW, drop current existing view
                 auto drop_ast = std::make_shared<ASTDropQuery>();
-                drop_ast->database = database_name;
+                drop_ast->database = create.database;
                 drop_ast->table = table_name;
                 drop_ast->no_ddl_lock = true;
 
@@ -619,7 +616,7 @@ bool InterpreterCreateQuery::doCreateTable(const ASTCreateQuery & create,
                 interpreter.execute();
             }
             else
-                throw Exception("Table " + database_name + "." + table_name + " already exists.", ErrorCodes::TABLE_ALREADY_EXISTS);
+                throw Exception("Table " + create.database + "." + table_name + " already exists.", ErrorCodes::TABLE_ALREADY_EXISTS);
         }
     }
     else if (context.tryGetExternalTable(table_name) && create.if_not_exists)
@@ -637,7 +634,7 @@ bool InterpreterCreateQuery::doCreateTable(const ASTCreateQuery & create,
         res = StorageFactory::instance().get(create,
             data_path,
             table_name,
-            database_name,
+            create.database,
             context,
             context.getGlobalContext(),
             properties.columns,
@@ -664,7 +661,7 @@ bool InterpreterCreateQuery::doCreateTable(const ASTCreateQuery & create,
     return true;
 }
 
-BlockIO InterpreterCreateQuery::fillTableIfNeeded(const ASTCreateQuery & create, const String & database_name)
+BlockIO InterpreterCreateQuery::fillTableIfNeeded(const ASTCreateQuery & create)
 {
     /// If the query is a CREATE SELECT, insert the data into the table.
     if (create.select && !create.attach
@@ -673,7 +670,7 @@ BlockIO InterpreterCreateQuery::fillTableIfNeeded(const ASTCreateQuery & create,
         auto insert = std::make_shared<ASTInsertQuery>();
 
         if (!create.temporary)
-            insert->database = database_name;
+            insert->database = create.database;
 
         insert->table = create.table;
         insert->select = create.select->clone();

dbms/src/Interpreters/InterpreterCreateQuery.h

@@ -67,9 +67,9 @@ private:
     void checkAccess(const ASTCreateQuery & create);
 
     /// Create IStorage and add it to database. If table already exists and IF NOT EXISTS specified, do nothing and return false.
-    bool doCreateTable(const ASTCreateQuery & create, const TableProperties & properties, const String & database_name);
+    bool doCreateTable(const ASTCreateQuery & create, const TableProperties & properties);
     /// Inserts data in created table if it's CREATE ... SELECT
-    BlockIO fillTableIfNeeded(const ASTCreateQuery & create, const String & database_name);
+    BlockIO fillTableIfNeeded(const ASTCreateQuery & create);
 
     ASTPtr query_ptr;
     Context & context;

docker/test/performance/run.sh

@@ -11,7 +11,7 @@ install_packages() {
 }
 
 download_data() {
-    clickhouse-client --query "CREATE DATABASE IF NOT EXISTS datasets"
+    clickhouse-client --query "ATTACH DATABASE IF NOT EXISTS datasets ENGINE = Ordinary"
     clickhouse-client --query "CREATE DATABASE IF NOT EXISTS test"
     /s3downloader --dataset-names $OPEN_DATASETS
     /s3downloader --dataset-names $PRIVATE_DATASETS --url 'https://s3.mds.yandex.net/clickhouse-private-datasets'

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" \
+    && 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" \

docker/test/stateful_with_coverage/run.sh

@@ -81,7 +81,7 @@ while /bin/true; do
 done &
 
 LLVM_PROFILE_FILE='client_%h_%p_%m.profraw' clickhouse-client --query "SHOW DATABASES"
-LLVM_PROFILE_FILE='client_%h_%p_%m.profraw' clickhouse-client --query "CREATE DATABASE datasets"
+LLVM_PROFILE_FILE='client_%h_%p_%m.profraw' clickhouse-client --query "ATTACH DATABASE datasets ENGINE = Ordinary"
 LLVM_PROFILE_FILE='client_%h_%p_%m.profraw' clickhouse-client --query "CREATE DATABASE test"
 
 kill_clickhouse

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" \
+    && 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" \