Skip to content

Commit a182a6b

Browse files
tavplubixtavplubix
authored
Merge pull request #45493 from azat/fix-detach
Fix possible in-use table after DETACH
2 parents c47a29a + ca8af3d commit a182a6b

7 files changed

Lines changed: 27 additions & 12 deletions

File tree

‎src/Databases/DatabaseOnDisk.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -377,14 +377,14 @@ void DatabaseOnDisk::renameTable(
377377
if (dictionary && table && !table->isDictionary())
378378
throw Exception("Use RENAME/EXCHANGE TABLE (instead of RENAME/EXCHANGE DICTIONARY) for tables", ErrorCodes::INCORRECT_QUERY);
379379

380+
table_lock = table->lockExclusively(
381+
local_context->getCurrentQueryId(), local_context->getSettingsRef().lock_acquire_timeout);
382+
380383
detachTable(local_context, table_name);
381384

382385
UUID prev_uuid = UUIDHelpers::Nil;
383386
try
384387
{
385-
table_lock = table->lockExclusively(
386-
local_context->getCurrentQueryId(), local_context->getSettingsRef().lock_acquire_timeout);
387-
388388
table_metadata_path = getObjectMetadataPath(table_name);
389389
attach_query = parseQueryFromMetadata(log, local_context, table_metadata_path);
390390
auto & create = attach_query->as<ASTCreateQuery &>();

‎src/Databases/DatabasesCommon.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ StoragePtr DatabaseWithOwnTablesBase::detachTableUnlocked(const String & table_n
233233
backQuote(database_name), backQuote(table_name));
234234
res = it->second;
235235
tables.erase(it);
236+
res->is_detached = true;
236237

237238
auto table_id = res->getStorageID();
238239
if (table_id.hasUUID())
@@ -269,6 +270,10 @@ void DatabaseWithOwnTablesBase::attachTableUnlocked(const String & table_name, c
269270
DatabaseCatalog::instance().removeUUIDMapping(table_id.uuid);
270271
throw Exception(ErrorCodes::TABLE_ALREADY_EXISTS, "Table {} already exists.", table_id.getFullTableName());
271272
}
273+
274+
/// It is important to reset is_detached here since in case of RENAME in
275+
/// non-Atomic database the is_detached is set to true before RENAME.
276+
table->is_detached = false;
272277
}
273278

274279
void DatabaseWithOwnTablesBase::shutdown()

‎src/Databases/MySQL/DatabaseMySQL.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,7 @@ void DatabaseMySQL::detachTablePermanently(ContextPtr, const String & table_name
448448
remove_or_detach_tables.erase(table_name);
449449
throw;
450450
}
451-
table_iter->second.second->is_dropped = true;
451+
table_iter->second.second->is_detached = true;
452452
}
453453

454454
void DatabaseMySQL::dropTable(ContextPtr local_context, const String & table_name, bool /*sync*/)

‎src/Interpreters/InterpreterDropQuery.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,10 @@ BlockIO InterpreterDropQuery::executeToTemporaryTable(const String & table_name,
287287
table->drop();
288288
table->is_dropped = true;
289289
}
290+
else if (kind == ASTDropQuery::Kind::Detach)
291+
{
292+
table->is_detached = true;
293+
}
290294
}
291295
}
292296

‎src/Storages/IStorage.cpp‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,10 @@ TableLockHolder IStorage::lockForShare(const String & query_id, const std::chron
5353
{
5454
TableLockHolder result = tryLockTimed(drop_lock, RWLockImpl::Read, query_id, acquire_timeout);
5555

56-
if (is_dropped)
56+
if (is_dropped || is_detached)
5757
{
5858
auto table_id = getStorageID();
59-
throw Exception(ErrorCodes::TABLE_IS_DROPPED, "Table {}.{} is dropped", table_id.database_name, table_id.table_name);
59+
throw Exception(ErrorCodes::TABLE_IS_DROPPED, "Table {}.{} is dropped or detached", table_id.database_name, table_id.table_name);
6060
}
6161
return result;
6262
}
@@ -65,7 +65,7 @@ TableLockHolder IStorage::tryLockForShare(const String & query_id, const std::ch
6565
{
6666
TableLockHolder result = tryLockTimed(drop_lock, RWLockImpl::Read, query_id, acquire_timeout);
6767

68-
if (is_dropped)
68+
if (is_dropped || is_detached)
6969
{
7070
// Table was dropped while acquiring the lock
7171
result = nullptr;
@@ -84,8 +84,11 @@ IStorage::AlterLockHolder IStorage::lockForAlter(const std::chrono::milliseconds
8484
"Possible deadlock avoided. Client should retry.",
8585
getStorageID().getFullTableName(), std::to_string(acquire_timeout.count()));
8686

87-
if (is_dropped)
88-
throw Exception("Table is dropped", ErrorCodes::TABLE_IS_DROPPED);
87+
if (is_dropped || is_detached)
88+
{
89+
auto table_id = getStorageID();
90+
throw Exception(ErrorCodes::TABLE_IS_DROPPED, "Table {}.{} is dropped or detached", table_id.database_name, table_id.table_name);
91+
}
8992

9093
return lock;
9194
}
@@ -96,8 +99,11 @@ TableExclusiveLockHolder IStorage::lockExclusively(const String & query_id, cons
9699
TableExclusiveLockHolder result;
97100
result.drop_lock = tryLockTimed(drop_lock, RWLockImpl::Write, query_id, acquire_timeout);
98101

99-
if (is_dropped)
100-
throw Exception("Table is dropped", ErrorCodes::TABLE_IS_DROPPED);
102+
if (is_dropped || is_detached)
103+
{
104+
auto table_id = getStorageID();
105+
throw Exception(ErrorCodes::TABLE_IS_DROPPED, "Table {}.{} is dropped or detached", table_id.database_name, table_id.table_name);
106+
}
101107

102108
return result;
103109
}

‎src/Storages/IStorage.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,7 @@ class IStorage : public std::enable_shared_from_this<IStorage>, public TypePromo
562562
virtual void onActionLockRemove(StorageActionBlockType /* action_type */) {}
563563

564564
std::atomic<bool> is_dropped{false};
565+
std::atomic<bool> is_detached{false};
565566

566567
/// Does table support index for IN sections
567568
virtual bool supportsIndexForIn() const { return false; }

‎src/Storages/LiveView/StorageLiveView.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,6 @@ void StorageLiveView::drop()
469469
DatabaseCatalog::instance().removeViewDependency(select_table_id, table_id);
470470

471471
std::lock_guard lock(mutex);
472-
is_dropped = true;
473472
condition.notify_all();
474473
}
475474

0 commit comments

Comments
 (0)