From 78f0d4c5cbd9ea4b04f31c3f7c20a122796f56d7 Mon Sep 17 00:00:00 2001
From: Alexander Tokmakov <avtokmakov@yandex-team.ru>
Date: Mon, 30 Mar 2020 22:15:18 +0300
Subject: [PATCH] check table finally detached before attaching

---
 dbms/src/Databases/DatabaseAtomic.cpp         | 24 +++++++++++++++++++
 dbms/src/Databases/DatabaseAtomic.h           |  7 +++---
 .../01107_atomic_db_detach_attach.reference   |  2 ++
 .../01107_atomic_db_detach_attach.sh          | 20 ++++++++++++++++
 4 files changed, 50 insertions(+), 3 deletions(-)
 create mode 100644 dbms/tests/queries/0_stateless/01107_atomic_db_detach_attach.reference
 create mode 100755 dbms/tests/queries/0_stateless/01107_atomic_db_detach_attach.sh

diff --git a/dbms/src/Databases/DatabaseAtomic.cpp b/dbms/src/Databases/DatabaseAtomic.cpp
index 72b7ae05dd8..97f8414ced6 100644
--- a/dbms/src/Databases/DatabaseAtomic.cpp
+++ b/dbms/src/Databases/DatabaseAtomic.cpp
@@ -52,6 +52,7 @@ void DatabaseAtomic::attachTable(const String & name, const StoragePtr & table,
 {
     assert(relative_table_path != data_path && !relative_table_path.empty());
     std::lock_guard lock(mutex);
+    assertDetachedTableNotInUse(table->getStorageID().uuid);
     DatabaseWithDictionaries::attachTableUnlocked(name, table, relative_table_path);
     table_name_to_path.emplace(std::make_pair(name, relative_table_path));
 }
@@ -61,6 +62,7 @@ StoragePtr DatabaseAtomic::detachTable(const String & name)
     std::lock_guard lock(mutex);
     auto table = DatabaseWithDictionaries::detachTableUnlocked(name);
     table_name_to_path.erase(name);
+    detached_tables.emplace(table->getStorageID().uuid, table);
     return table;
 }
 
@@ -138,6 +140,7 @@ void DatabaseAtomic::loadStoredObjects(Context & context, bool has_force_restore
 
 void DatabaseAtomic::shutdown()
 {
+
     DatabaseWithDictionaries::shutdown();
 }
 
@@ -148,6 +151,7 @@ void DatabaseAtomic::commitCreateTable(const ASTCreateQuery & query, const Stora
     try
     {
         std::lock_guard lock{mutex};
+        assertDetachedTableNotInUse(query.uuid);
         renameNoReplace(table_metadata_tmp_path, table_metadata_path);
         attachTableUnlocked(query.table, table, table_data_path);   /// Should never throw
         table_name_to_path.emplace(query.table, table_data_path);
@@ -173,5 +177,25 @@ void DatabaseAtomic::commitAlterTable(const StorageID & table_id, const String &
     renameExchange(table_metadata_tmp_path, table_metadata_path);
 }
 
+void DatabaseAtomic::assertDetachedTableNotInUse(const UUID & uuid)
+{
+    cleenupDetachedTables();
+    if (detached_tables.count(uuid))
+        throw Exception("Cannot attach table with UUID " + toString(uuid) +
+              ", because it was detached but still used by come query. Retry later.", ErrorCodes::TABLE_ALREADY_EXISTS);
+}
+
+void DatabaseAtomic::cleenupDetachedTables()
+{
+    auto it = detached_tables.begin();
+    while (it != detached_tables.end())
+    {
+        if (it->second.unique())
+            it = detached_tables.erase(it);
+        else
+            ++it;
+    }
+}
+
 }
 
diff --git a/dbms/src/Databases/DatabaseAtomic.h b/dbms/src/Databases/DatabaseAtomic.h
index c1c0a8fc75a..f2d93167cab 100644
--- a/dbms/src/Databases/DatabaseAtomic.h
+++ b/dbms/src/Databases/DatabaseAtomic.h
@@ -22,7 +22,6 @@ public:
             IDatabase & to_database,
             const String & to_table_name) override;
 
-    //void removeTable(const Context & context, const String & table_name) override;
     void dropTable(const Context & context, const String & table_name, bool no_delay) override;
 
     void attachTable(const String & name, const StoragePtr & table, const String & relative_table_path) override;
@@ -47,11 +46,13 @@ private:
     void commitCreateTable(const ASTCreateQuery & query, const StoragePtr & table,
                            const String & table_metadata_tmp_path, const String & table_metadata_path) override;
 
+    void assertDetachedTableNotInUse(const UUID & uuid);
+    void cleenupDetachedTables();
+
     //TODO store path in DatabaseWithOwnTables::tables
     std::map<String, String> table_name_to_path;
 
-
-
+    std::map<UUID, StoragePtr> detached_tables;
 };
 
 }
diff --git a/dbms/tests/queries/0_stateless/01107_atomic_db_detach_attach.reference b/dbms/tests/queries/0_stateless/01107_atomic_db_detach_attach.reference
new file mode 100644
index 00000000000..a08773dab53
--- /dev/null
+++ b/dbms/tests/queries/0_stateless/01107_atomic_db_detach_attach.reference
@@ -0,0 +1,2 @@
+OK
+5	10
diff --git a/dbms/tests/queries/0_stateless/01107_atomic_db_detach_attach.sh b/dbms/tests/queries/0_stateless/01107_atomic_db_detach_attach.sh
new file mode 100755
index 00000000000..e0b46c4e7b7
--- /dev/null
+++ b/dbms/tests/queries/0_stateless/01107_atomic_db_detach_attach.sh
@@ -0,0 +1,20 @@
+#!/usr/bin/env bash
+
+CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
+. $CURDIR/../shell_config.sh
+
+$CLICKHOUSE_CLIENT -q "DROP DATABASE IF EXISTS test_01107"
+$CLICKHOUSE_CLIENT -q "CREATE DATABASE test_01107 ENGINE=Atomic"
+$CLICKHOUSE_CLIENT -q "CREATE TABLE test_01107.mt (n UInt64) ENGINE=MergeTree() ORDER BY tuple()"
+
+$CLICKHOUSE_CLIENT -q "INSERT INTO test_01107.mt SELECT number + sleepEachRow(1) FROM numbers(5)" &
+sleep 1
+
+$CLICKHOUSE_CLIENT -q "DETACH TABLE test_01107.mt"
+$CLICKHOUSE_CLIENT -q "ATTACH TABLE test_01107.mt" 2>&1 | grep -F "Code: 57" > /dev/null && echo "OK"
+
+sleep 5
+$CLICKHOUSE_CLIENT -q "ATTACH TABLE test_01107.mt"
+$CLICKHOUSE_CLIENT -q "SELECT count(n), sum(n) FROM test_01107.mt"
+
+$CLICKHOUSE_CLIENT -q "DROP DATABASE test_01107"