fix race between DROP and short form of ATTACH

This commit is contained in:
Alexander Tokmakov 2022-09-14 18:03:36 +02:00
parent baf7255cff
commit fc73134145
3 changed files with 27 additions and 10 deletions

View File

@ -185,6 +185,7 @@ void DatabaseOnDisk::createTable(
if (create.attach_short_syntax)
{
/// Metadata already exists, table was detached
assert(fs::exists(getObjectMetadataPath(table_name)));
removeDetachedPermanentlyFlag(local_context, table_name, table_metadata_path, true);
attachTable(local_context, table_name, table, getTableDataPath(create));
return;

View File

@ -1001,6 +1001,8 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create)
String current_database = getContext()->getCurrentDatabase();
auto database_name = create.database ? create.getDatabase() : current_database;
DDLGuardPtr ddl_guard;
// If this is a stub ATTACH query, read the query definition from the database
if (create.attach && !create.storage && !create.columns_list)
{
@ -1022,6 +1024,10 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create)
if (!create.cluster.empty())
return executeQueryOnCluster(create);
/// For short syntax of ATTACH query we have to lock table name here, before reading metadata
/// and hold it until table is attached
ddl_guard = DatabaseCatalog::instance().getDDLGuard(database_name, create.getTable());
bool if_not_exists = create.if_not_exists;
// Table SQL definition is available even if the table is detached (even permanently)
@ -1053,6 +1059,7 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create)
if (create.attach_from_path)
{
chassert(!ddl_guard);
fs::path user_files = fs::path(getContext()->getUserFilesPath()).lexically_normal();
fs::path root_path = fs::path(getContext()->getPath()).lexically_normal();
@ -1147,6 +1154,7 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create)
if (need_add_to_database && database->getEngineName() == "Replicated")
{
chassert(!ddl_guard);
auto guard = DatabaseCatalog::instance().getDDLGuard(create.getDatabase(), create.getTable());
if (auto * ptr = typeid_cast<DatabaseReplicated *>(database.get());
@ -1159,13 +1167,20 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create)
}
if (!create.cluster.empty())
{
chassert(!ddl_guard);
return executeQueryOnCluster(create);
}
if (create.replace_table)
{
chassert(!ddl_guard);
return doCreateOrReplaceTable(create, properties);
}
/// Actually creates table
bool created = doCreateTable(create, properties);
bool created = doCreateTable(create, properties, ddl_guard);
ddl_guard.reset();
if (!created) /// Table already exists
return {};
@ -1180,7 +1195,8 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create)
}
bool InterpreterCreateQuery::doCreateTable(ASTCreateQuery & create,
const InterpreterCreateQuery::TableProperties & properties)
const InterpreterCreateQuery::TableProperties & properties,
DDLGuardPtr & ddl_guard)
{
if (create.temporary)
{
@ -1193,16 +1209,12 @@ bool InterpreterCreateQuery::doCreateTable(ASTCreateQuery & create,
return true;
}
std::unique_ptr<DDLGuard> guard;
if (!ddl_guard)
ddl_guard = DatabaseCatalog::instance().getDDLGuard(create.getDatabase(), create.getTable());
String data_path;
DatabasePtr database;
/** If the request specifies IF NOT EXISTS, we allow concurrent CREATE queries (which do nothing).
* If table doesn't exist, one thread is creating table, while others wait in DDLGuard.
*/
guard = DatabaseCatalog::instance().getDDLGuard(create.getDatabase(), create.getTable());
database = DatabaseCatalog::instance().getDatabase(create.getDatabase());
assertOrSetUUID(create, database);
@ -1411,7 +1423,9 @@ BlockIO InterpreterCreateQuery::doCreateOrReplaceTable(ASTCreateQuery & create,
try
{
/// Create temporary table (random name will be generated)
[[maybe_unused]] bool done = InterpreterCreateQuery(query_ptr, create_context).doCreateTable(create, properties);
DDLGuardPtr ddl_guard;
[[maybe_unused]] bool done = InterpreterCreateQuery(query_ptr, create_context).doCreateTable(create, properties, ddl_guard);
ddl_guard.reset();
assert(done);
created = true;

View File

@ -18,7 +18,9 @@ class ASTExpressionList;
class ASTConstraintDeclaration;
class ASTStorage;
class IDatabase;
class DDLGuard;
using DatabasePtr = std::shared_ptr<IDatabase>;
using DDLGuardPtr = std::unique_ptr<DDLGuard>;
/** Allows to create new table or database,
@ -89,7 +91,7 @@ private:
AccessRightsElements getRequiredAccess() const;
/// Create IStorage and add it to database. If table already exists and IF NOT EXISTS specified, do nothing and return false.
bool doCreateTable(ASTCreateQuery & create, const TableProperties & properties);
bool doCreateTable(ASTCreateQuery & create, const TableProperties & properties, DDLGuardPtr & ddl_guard);
BlockIO doCreateOrReplaceTable(ASTCreateQuery & create, const InterpreterCreateQuery::TableProperties & properties);
/// Inserts data in created table if it's CREATE ... SELECT
BlockIO fillTableIfNeeded(const ASTCreateQuery & create);