revert some changes around DDLGuard

dbms/src/Interpreters/DatabaseCatalog.cpp

@@ -139,13 +139,6 @@ void DatabaseCatalog::shutdown()
     for (auto & database : current_databases)
         database.second->shutdown();
 
-    {
-        std::lock_guard lock(tables_marked_droped_mutex);
-        for (auto & elem : tables_marked_droped)
-            if (elem.need_shutdown)
-                elem.table->shutdown();
-    }
-
     std::lock_guard lock(databases_mutex);
     assert(std::find_if_not(uuid_map.begin(), uuid_map.end(), [](const auto & elem) { return elem.map.empty(); }) == uuid_map.end());
     databases.clear();
@@ -482,7 +475,7 @@ void DatabaseCatalog::loadMarkedAsDroppedTables()
         String full_path = path + it.name();
 
         Strings name_parts;
-        boost::split(name_parts, it.name(), boost::is_any_of("."));
+        boost::split(name_parts, it.name(), boost::is_any_of(".")); // NOLINT: LLVM Bug 41141
         if (name_parts.size() != 4)     /// Unexpected file
             continue;
 
@@ -520,7 +513,6 @@ void DatabaseCatalog::enqueueDroppedTableCleanup(StorageID table_id, StoragePtr
     assert(dropped_metadata_path == getPathForDroppedMetadata(table_id));
 
     time_t drop_time;
-    bool need_shutdown = true;
     if (table)
         drop_time = std::chrono::system_clock::to_time_t(std::chrono::system_clock::now());
     else
@@ -557,14 +549,13 @@ void DatabaseCatalog::enqueueDroppedTableCleanup(StorageID table_id, StoragePtr
         }
 
         drop_time = Poco::File(dropped_metadata_path).getLastModified().epochTime();
-        need_shutdown = false;
     }
 
     std::lock_guard lock(tables_marked_droped_mutex);
     if (ignore_delay)
-        tables_marked_droped.push_front({table_id, table, dropped_metadata_path, 0, need_shutdown});
+        tables_marked_droped.push_front({table_id, table, dropped_metadata_path, 0});
     else
-        tables_marked_droped.push_back({table_id, table, dropped_metadata_path, drop_time, need_shutdown});
+        tables_marked_droped.push_back({table_id, table, dropped_metadata_path, drop_time});
 }
 
 void DatabaseCatalog::dropTableDataTask()
@@ -583,19 +574,14 @@ void DatabaseCatalog::dropTableDataTask()
             //              "time elapsed = " + std::to_string(current_time - elem.drop_time));
             bool not_in_use = !elem.table || elem.table.unique();
             bool old_enough = elem.drop_time + drop_delay_s < current_time;
-            return (not_in_use && old_enough) || elem.need_shutdown;
+            return not_in_use && old_enough;
         });
-        if (it != tables_marked_droped.end() && !it->need_shutdown)
+        if (it != tables_marked_droped.end())
         {
             table = std::move(*it);
             LOG_INFO(log, "Will try drop " + table.table_id.getNameForLogs());
             tables_marked_droped.erase(it);
         }
-        else if (it->need_shutdown)
-        {
-            table = *it;
-            it->need_shutdown = false;
-        }
     }
     catch (...)
     {
@@ -604,12 +590,6 @@ void DatabaseCatalog::dropTableDataTask()
 
     if (table.table_id)
     {
-        if (table.need_shutdown)
-        {
-            table.table->shutdown();
-            (*drop_task)->scheduleAfter(0);
-            return;
-        }
 
         try
         {

dbms/src/Interpreters/DatabaseCatalog.h

@@ -185,7 +185,6 @@ private:
         StoragePtr table;
         String metadata_path;
         time_t drop_time;
-        bool need_shutdown{true};
     };
     using TablesMarkedAsDropped = std::list<TableMarkedAsDropped>;
 

dbms/src/Interpreters/InterpreterCreateQuery.cpp

@@ -539,26 +539,13 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create)
 
     String current_database = context.getCurrentDatabase();
 
-    String metadata_path_tmp;
-    bool created_tmp = false;
-    SCOPE_EXIT({ if (created_tmp) Poco::File(metadata_path_tmp).remove(); });
-
     // If this is a stub ATTACH query, read the query definition from the database
     if (create.attach && !create.storage && !create.columns_list)
     {
         auto database_name = create.database.empty() ? current_database : create.database;
         auto database = DatabaseCatalog::instance().getDatabase(database_name);
-
-        //TODO do we really need it? refactor it if we do
-        if (database->getEngineName() == "Atomic")
-        {
-            metadata_path_tmp = database->getObjectMetadataPath(create.table) + ".tmp";
-            if (!Poco::File(metadata_path_tmp).createFile())
-                throw Exception("Cannot attach table because of concurrent query.", ErrorCodes::TABLE_ALREADY_EXISTS);
-            created_tmp = true;
-        }
-
         bool if_not_exists = create.if_not_exists;
+
         // Table SQL definition is available even if the table is detached
         auto query = database->getCreateTableQuery(context, create.table);
         create = query->as<ASTCreateQuery &>(); // Copy the saved create query, but use ATTACH instead of CREATE
@@ -622,8 +609,7 @@ bool InterpreterCreateQuery::doCreateTable(/*const*/ ASTCreateQuery & create,
         /** 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.
           */
-        if (database->getEngineName() != "Atomic")
+        guard = DatabaseCatalog::instance().getDDLGuard(create.database, table_name);
-            guard = DatabaseCatalog::instance().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))

dbms/src/Interpreters/InterpreterDropQuery.cpp

@@ -75,10 +75,7 @@ BlockIO InterpreterDropQuery::executeToTable(
 
     auto table_id = context.resolveStorageID(table_id_, Context::ResolveOrdinary);
 
-    std::unique_ptr<DDLGuard> ddl_guard;
+    auto ddl_guard = (!query.no_ddl_lock ? DatabaseCatalog::instance().getDDLGuard(table_id.database_name, table_id.table_name) : nullptr);
-    if (auto db = DatabaseCatalog::instance().tryGetDatabase(table_id.database_name))   //FIXME
-        if (db->getEngineName() != "Atomic")
-            ddl_guard = (!query.no_ddl_lock ? DatabaseCatalog::instance().getDDLGuard(table_id.database_name, table_id.table_name) : nullptr);
 
     auto [database, table] = tryGetDatabaseAndTable(table_id.database_name, table_id.table_name, query.if_exists);
 
@@ -115,12 +112,11 @@ BlockIO InterpreterDropQuery::executeToTable(
             context.checkAccess(table->isView() ? AccessType::DROP_VIEW : AccessType::DROP_TABLE, table_id);
             table->checkTableCanBeDropped();
 
+            table->shutdown();
+
             TableStructureWriteLockHolder table_lock;
             if (database->getEngineName() != "Atomic")
-            {
-                table->shutdown();
                 table_lock = table->lockExclusively(context.getCurrentQueryId());
-            }
 
             database->dropTable(context, table_id.table_name, query.no_delay);
         }