ISSUES-2259 add comment & move delete database metadata implementation

dbms/src/Common/ZooKeeper/Types.h

@@ -54,8 +54,10 @@ using SetResponse = ZooKeeperImpl::ZooKeeper::SetResponse;
 using ListResponse = ZooKeeperImpl::ZooKeeper::ListResponse;
 using CheckResponse = ZooKeeperImpl::ZooKeeper::CheckResponse;
 
+/// Gets multiple asynchronous results
+/// Each pair, the first is path, the second is response eg. CreateResponse, RemoveResponse
 template <typename R>
-using MultiAsyncResponse = std::vector<std::pair<std::string, std::future<R>>>;
+using AsyncResponses = std::vector<std::pair<std::string, std::future<R>>>;
 
 RequestPtr makeCreateRequest(const std::string & path, const std::string & data, int create_mode);
 RequestPtr makeRemoveRequest(const std::string & path, int version);

dbms/src/Databases/DatabaseDictionary.cpp

@@ -213,4 +213,9 @@ void DatabaseDictionary::shutdown()
 {
 }
 
+String DatabaseDictionary::getDatabaseName() const
+{
+    return name;
+}
+
 }

dbms/src/Databases/DatabaseDictionary.h

@@ -25,6 +25,8 @@ class DatabaseDictionary : public IDatabase
 public:
     DatabaseDictionary(const String & name_, const Context & context);
 
+    String getDatabaseName() const override;
+
     String getEngineName() const override
     {
         return "Dictionary";

dbms/src/Databases/DatabaseMemory.cpp

@@ -78,4 +78,9 @@ ASTPtr DatabaseMemory::getCreateDatabaseQuery(
     throw Exception("There is no CREATE DATABASE query for DatabaseMemory", ErrorCodes::CANNOT_GET_CREATE_TABLE_QUERY);
 }
 
+String DatabaseMemory::getDatabaseName() const
+{
+    return name;
+}
+
 }

dbms/src/Databases/DatabaseMemory.h

@@ -19,6 +19,8 @@ class DatabaseMemory : public DatabaseWithOwnTablesBase
 public:
     DatabaseMemory(String name_);
 
+    String getDatabaseName() const override;
+
     String getEngineName() const override { return "Memory"; }
 
     void loadTables(

dbms/src/Databases/DatabaseOrdinary.cpp

@@ -570,6 +570,11 @@ String DatabaseOrdinary::getMetadataPath() const
     return metadata_path;
 }
 
+String DatabaseOrdinary::getDatabaseName() const
+{
+    return name;
+}
+
 String DatabaseOrdinary::getTableMetadataPath(const String & table_name) const
 {
     return detail::getTableMetadataPath(metadata_path, table_name);

dbms/src/Databases/DatabaseOrdinary.h

@@ -59,6 +59,7 @@ public:
     ASTPtr getCreateDatabaseQuery(const Context & context) const override;
 
     String getDataPath() const override;
+    String getDatabaseName() const override;
     String getMetadataPath() const override;
     String getTableMetadataPath(const String & table_name) const override;
 

dbms/src/Databases/IDatabase.h

@@ -7,6 +7,8 @@
 #include <memory>
 #include <functional>
 #include <Poco/File.h>
+#include <Common/escapeForFileName.h>
+#include <Interpreters/Context.h>
 
 
 class ThreadPool;
@@ -133,6 +135,7 @@ public:
     /// Get the CREATE DATABASE query for current database.
     virtual ASTPtr getCreateDatabaseQuery(const Context & context) const = 0;
 
+    virtual String getDatabaseName() const { return {}; }
     /// Returns path for persistent data storage if the database supports it, empty string otherwise
     virtual String getDataPath() const { return {}; }
     /// Returns metadata path if the database supports it, empty string otherwise
@@ -144,11 +147,17 @@ public:
     virtual void shutdown() = 0;
 
     /// Delete metadata, the deletion of which differs from the recursive deletion of the directory, if any.
-    virtual void drop()
+    virtual void drop(Context & context)
     {
-        String metadata_path = getMetadataPath();
-        if (!metadata_path.empty())
-            Poco::File(metadata_path).remove(false);
+        String database_name = getDatabaseName();
+        String database_name_escaped = escapeForFileName(database_name);
+
+        Poco::File(context.getPath() + "metadata/" + database_name_escaped + "/").remove(false);
+
+        /// Old ClickHouse versions did not store database.sql files
+        Poco::File database_metadata_file(context.getPath() + "metadata/" + database_name_escaped + ".sql");
+        if (database_metadata_file.exists())
+            database_metadata_file.remove(false);
     };
 
     virtual ~IDatabase() {}

dbms/src/Interpreters/InterpreterDropQuery.cpp

@@ -168,17 +168,12 @@ BlockIO InterpreterDropQuery::executeToDatabase(String & database_name, ASTDropQ
 
             database->shutdown();
             /// Delete the database.
-            database->drop();
+            database->drop(context);
 
             /// Remove data directory if it is not virtual database. TODO: should IDatabase::drop() do that?
             String database_data_path = database->getDataPath();
             if (!database_data_path.empty())
                 Poco::File(database_data_path).remove(false);
-
-            /// Old ClickHouse versions did not store database.sql files
-            Poco::File database_metadata_file(context.getPath() + "metadata/" + escapeForFileName(database_name) + ".sql");
-            if (database_metadata_file.exists())
-                database_metadata_file.remove(false);
         }
     }
 

dbms/src/Storages/MergeTree/ReplicatedMergeTreeCleanupThread.cpp

@@ -157,7 +157,7 @@ void ReplicatedMergeTreeCleanupThread::clearOldBlocks()
     auto first_outdated_block_time_threshold = std::upper_bound(timed_blocks.begin(), timed_blocks.end(), block_threshold, NodeWithStat::greaterByTime);
     auto first_outdated_block = std::min(first_outdated_block_fixed_threshold, first_outdated_block_time_threshold);
 
-    zkutil::MultiAsyncResponse<zkutil::RemoveResponse> try_remove_futures;
+    zkutil::AsyncResponses<zkutil::RemoveResponse> try_remove_futures;
     for (auto it = first_outdated_block; it != timed_blocks.end(); ++it)
     {
         String path = storage.zookeeper_path + "/blocks/" + it->node;
@@ -212,7 +212,7 @@ void ReplicatedMergeTreeCleanupThread::getBlocksSortedByTime(zkutil::ZooKeeper &
                                    << " to clear old ones from ZooKeeper.");
     }
 
-    zkutil::MultiAsyncResponse<zkutil::ExistsResponse> exists_futures;
+    zkutil::AsyncResponses<zkutil::ExistsResponse> exists_futures;
     for (const String & block : blocks)
     {
         auto it = cached_block_stats.find(block);

dbms/src/Storages/MergeTree/ReplicatedMergeTreeQueue.cpp

@@ -53,7 +53,7 @@ bool ReplicatedMergeTreeQueue::load(zkutil::ZooKeeperPtr zookeeper)
 
         std::sort(children.begin(), children.end());
 
-        zkutil::MultiAsyncResponse<zkutil::GetResponse> futures;
+        zkutil::AsyncResponses<zkutil::GetResponse> futures;
         futures.reserve(children.size());
 
         for (const String & child : children)
@@ -315,7 +315,7 @@ bool ReplicatedMergeTreeQueue::pullLogsToQueue(zkutil::ZooKeeperPtr zookeeper, B
 
             LOG_DEBUG(log, "Pulling " << (end - begin) << " entries to queue: " << *begin << " - " << *last);
 
-            zkutil::MultiAsyncResponse<zkutil::GetResponse> futures;
+            zkutil::AsyncResponses<zkutil::GetResponse> futures;
             futures.reserve(end - begin);
 
             for (auto it = begin; it != end; ++it)

dbms/src/Storages/StorageReplicatedMergeTree.cpp

@@ -3736,7 +3736,7 @@ void StorageReplicatedMergeTree::clearBlocksInPartition(
         throw Exception(zookeeper_path + "/blocks doesn't exist", ErrorCodes::NOT_FOUND_NODE);
 
     String partition_prefix = partition_id + "_";
-    zkutil::MultiAsyncResponse<zkutil::GetResponse> get_futures;
+    zkutil::AsyncResponses<zkutil::GetResponse> get_futures;
     for (const String & block_id : blocks)
     {
         if (startsWith(block_id, partition_prefix))
@@ -3746,7 +3746,7 @@ void StorageReplicatedMergeTree::clearBlocksInPartition(
         }
     }
 
-    zkutil::MultiAsyncResponse<zkutil::RemoveResponse> to_delete_futures;
+    zkutil::AsyncResponses<zkutil::RemoveResponse> to_delete_futures;
     for (auto & pair : get_futures)
     {
         const String & path = pair.first;