From b6039f8c50a13077b76ee6a6516292c8c9419ccf Mon Sep 17 00:00:00 2001 From: Alexander Tokmakov Date: Wed, 12 Feb 2020 21:14:12 +0300 Subject: [PATCH] remove tryGetExternalTable --- dbms/programs/server/TCPHandler.cpp | 16 +++--- dbms/src/Interpreters/Context.cpp | 52 ++++++++++++------- dbms/src/Interpreters/Context.h | 18 +++---- dbms/src/Interpreters/DatabaseCatalog.cpp | 12 +++-- dbms/src/Interpreters/DatabaseCatalog.h | 2 +- dbms/src/Interpreters/ExternalTablesVisitor.h | 4 +- .../src/Interpreters/InterpreterDropQuery.cpp | 5 +- dbms/src/Storages/StorageID.h | 5 ++ 8 files changed, 70 insertions(+), 44 deletions(-) diff --git a/dbms/programs/server/TCPHandler.cpp b/dbms/programs/server/TCPHandler.cpp index 3cb00320b10..1174f3c465e 100644 --- a/dbms/programs/server/TCPHandler.cpp +++ b/dbms/programs/server/TCPHandler.cpp @@ -960,8 +960,8 @@ bool TCPHandler::receiveData(bool scalar) initBlockInput(); /// The name of the temporary table for writing data, default to empty string - String name; - readStringBinary(name, *in); + auto temporary_id = StorageID::createEmpty(); + readStringBinary(temporary_id.table_name, *in); /// Read one block from the network and write it down Block block = state.block_in->read(); @@ -969,22 +969,24 @@ bool TCPHandler::receiveData(bool scalar) if (block) { if (scalar) - query_context->addScalar(name, block); + query_context->addScalar(temporary_id.table_name, block); else { /// If there is an insert request, then the data should be written directly to `state.io.out`. /// Otherwise, we write the blocks in the temporary `external_table_name` table. if (!state.need_receive_data_for_insert && !state.need_receive_data_for_input) { + auto resolved = query_context->tryResolveStorageID(temporary_id, Context::ResolveExternal); StoragePtr storage; /// If such a table does not exist, create it. - if (!(storage = query_context->tryGetExternalTable(name))) + if (temporary_id.empty()) { NamesAndTypesList columns = block.getNamesAndTypesList(); - storage = StorageMemory::create(StorageID("_external", name), ColumnsDescription{columns}, ConstraintsDescription{}); + storage = StorageMemory::create(temporary_id, ColumnsDescription{columns}, ConstraintsDescription{}); storage->startup(); - query_context->addExternalTable(name, storage); - } + query_context->addExternalTable(temporary_id.table_name, storage); + } else + storage = DatabaseCatalog::instance().getTable(resolved, *query_context); /// The data will be written directly to the table. state.io.out = storage->write(ASTPtr(), *query_context); } diff --git a/dbms/src/Interpreters/Context.cpp b/dbms/src/Interpreters/Context.cpp index eac208c7086..54b50537ae5 100644 --- a/dbms/src/Interpreters/Context.cpp +++ b/dbms/src/Interpreters/Context.cpp @@ -718,7 +718,7 @@ void Context::setUser(const String & name, const String & password, const Poco:: bool Context::isTableExist(const String & database_name, const String & table_name) const { - auto table_id = resolveStorageID({database_name, table_name}, StorageNamespace::Ordinary); + auto table_id = resolveStorageID({database_name, table_name}, StorageNamespace::ResolveOrdinary); return DatabaseCatalog::instance().isTableExist(table_id, *this); } @@ -772,15 +772,6 @@ Tables Context::getExternalTables() const } -StoragePtr Context::tryGetExternalTable(const String & table_name) const -{ - auto it = external_tables_mapping.find(table_name); - if (external_tables_mapping.end() == it) - return StoragePtr(); - - return it->second->getTable(); -} - StoragePtr Context::getTable(const String & database_name, const String & table_name) const { return getTable(StorageID(database_name, table_name)); @@ -1965,24 +1956,43 @@ void Context::resetInputCallbacks() StorageID Context::resolveStorageID(StorageID storage_id, StorageNamespace where) const { auto lock = getLock(); - return resolveStorageIDUnlocked(std::move(storage_id), where); + std::optional exc; + auto resolved = resolveStorageIDImpl(std::move(storage_id), where, &exc); + if (exc) + throw *exc; + return resolved; } -StorageID Context::resolveStorageIDUnlocked(StorageID storage_id, StorageNamespace where) const +StorageID Context::tryResolveStorageID(StorageID storage_id, StorageNamespace where) const +{ + auto lock = getLock(); + return resolveStorageIDImpl(std::move(storage_id), where, nullptr); +} + +StorageID Context::resolveStorageIDImpl(StorageID storage_id, StorageNamespace where, std::optional * exception) const { if (storage_id.uuid != UUIDHelpers::Nil) return storage_id; - bool look_for_external_table = where & StorageNamespace::External; - bool in_current_database = where & StorageNamespace::CurrentDatabase; - bool in_specified_database = where & StorageNamespace::Global; + if (storage_id.empty()) + { + if (exception) + exception->emplace("Both table name and UUID are empty", ErrorCodes::UNKNOWN_TABLE); + return storage_id; + } + + bool look_for_external_table = where & StorageNamespace::ResolveExternal; + bool in_current_database = where & StorageNamespace::ResolveCurrentDatabase; + bool in_specified_database = where & StorageNamespace::ResolveGlobal; if (!storage_id.database_name.empty()) { if (in_specified_database) return storage_id; - throw Exception("External and temporary tables have no database, but " + + if (exception) + exception->emplace("External and temporary tables have no database, but " + storage_id.database_name + " is specified", ErrorCodes::UNKNOWN_TABLE); + return StorageID::createEmpty(); } if (look_for_external_table) @@ -1995,12 +2005,18 @@ StorageID Context::resolveStorageIDUnlocked(StorageID storage_id, StorageNamespa if (in_current_database) { if (current_database.empty()) - throw Exception("Default database is not selected", ErrorCodes::UNKNOWN_DATABASE); + { + if (exception) + exception->emplace("Default database is not selected", ErrorCodes::UNKNOWN_DATABASE); + return StorageID::createEmpty(); + } storage_id.database_name = current_database; return storage_id; } - throw Exception("Cannot resolve database name for table " + storage_id.getNameForLogs(), ErrorCodes::UNKNOWN_TABLE); + if (exception) + exception->emplace("Cannot resolve database name for table " + storage_id.getNameForLogs(), ErrorCodes::UNKNOWN_TABLE); + return StorageID::createEmpty(); } diff --git a/dbms/src/Interpreters/Context.h b/dbms/src/Interpreters/Context.h index a4cfdf7b70d..df482c43b1d 100644 --- a/dbms/src/Interpreters/Context.h +++ b/dbms/src/Interpreters/Context.h @@ -287,22 +287,22 @@ public: enum StorageNamespace { - Global = 1u, /// Database name must be specified - CurrentDatabase = 2u, /// Use current database - Ordinary = Global | CurrentDatabase, /// If database name is not specified, use current database - External = 4u, /// Try get external table - All = External | Ordinary /// If database name is not specified, try get external table, - /// if external table not found use current database. + ResolveGlobal = 1u, /// Database name must be specified + ResolveCurrentDatabase = 2u, /// Use current database + ResolveOrdinary = ResolveGlobal | ResolveCurrentDatabase, /// If database name is not specified, use current database + ResolveExternal = 4u, /// Try get external table + ResolveAll = ResolveExternal | ResolveOrdinary /// If database name is not specified, try get external table, + /// if external table not found use current database. }; String resolveDatabase(const String & database_name) const; - StorageID resolveStorageID(StorageID storage_id, StorageNamespace where = StorageNamespace::All) const; - StorageID resolveStorageIDUnlocked(StorageID storage_id, StorageNamespace where = StorageNamespace::All) const; + StorageID resolveStorageID(StorageID storage_id, StorageNamespace where = StorageNamespace::ResolveAll) const; + StorageID tryResolveStorageID(StorageID storage_id, StorageNamespace where = StorageNamespace::ResolveAll) const; + StorageID resolveStorageIDImpl(StorageID storage_id, StorageNamespace where, std::optional * exception) const; const Scalars & getScalars() const; const Block & getScalar(const String & name) const; Tables getExternalTables() const; - StoragePtr tryGetExternalTable(const String & table_name) const; StoragePtr getTable(const String & database_name, const String & table_name) const; StoragePtr getTable(const StorageID & table_id) const; StoragePtr tryGetTable(const String & database_name, const String & table_name) const; diff --git a/dbms/src/Interpreters/DatabaseCatalog.cpp b/dbms/src/Interpreters/DatabaseCatalog.cpp index d3a33190e1b..3f3851b68e3 100644 --- a/dbms/src/Interpreters/DatabaseCatalog.cpp +++ b/dbms/src/Interpreters/DatabaseCatalog.cpp @@ -243,20 +243,22 @@ DatabasePtr DatabaseCatalog::getDatabase(const String & database_name, const Con void DatabaseCatalog::addDependency(const StorageID & from, const StorageID & where) { std::lock_guard lock{databases_mutex}; - view_dependencies[from].insert(where); + // FIXME when loading metadata storage may not know UUIDs of it's dependencies, because they are not loaded yet, + // so UUID of `from` is not used here. (same for remove, get and update) + view_dependencies[{from.getDatabaseName(), from.getTableName()}].insert(where); } void DatabaseCatalog::removeDependency(const StorageID & from, const StorageID & where) { std::lock_guard lock{databases_mutex}; - view_dependencies[from].erase(where); + view_dependencies[{from.getDatabaseName(), from.getTableName()}].erase(where); } Dependencies DatabaseCatalog::getDependencies(const StorageID & from) const { std::lock_guard lock{databases_mutex}; - auto iter = view_dependencies.find(from); + auto iter = view_dependencies.find({from.getDatabaseName(), from.getTableName()}); if (iter == view_dependencies.end()) return {}; return Dependencies(iter->second.begin(), iter->second.end()); @@ -268,9 +270,9 @@ DatabaseCatalog::updateDependency(const StorageID & old_from, const StorageID & { std::lock_guard lock{databases_mutex}; if (!old_from.empty()) - view_dependencies[old_from].erase(old_where); + view_dependencies[{old_from.getDatabaseName(), old_from.getTableName()}].erase(old_where); if (!new_from.empty()) - view_dependencies[new_from].insert(new_where); + view_dependencies[{new_from.getDatabaseName(), new_from.getTableName()}].insert(new_where); } std::unique_ptr DatabaseCatalog::getDDLGuard(const String & database, const String & table) diff --git a/dbms/src/Interpreters/DatabaseCatalog.h b/dbms/src/Interpreters/DatabaseCatalog.h index b3c3963c047..09690c79db0 100644 --- a/dbms/src/Interpreters/DatabaseCatalog.h +++ b/dbms/src/Interpreters/DatabaseCatalog.h @@ -96,7 +96,7 @@ public: void addUUIDMapping(const UUID & uuid, DatabasePtr database, StoragePtr table); void removeUUIDMapping(const UUID & uuid); - StoragePtr getTable(const StorageID & table_id, const Context & local_context, std::optional * exception) const; + StoragePtr getTable(const StorageID & table_id, const Context & local_context, std::optional * exception = nullptr) const; void addDependency(const StorageID & from, const StorageID & where); diff --git a/dbms/src/Interpreters/ExternalTablesVisitor.h b/dbms/src/Interpreters/ExternalTablesVisitor.h index 3ff5f38f84d..cd05d61a365 100644 --- a/dbms/src/Interpreters/ExternalTablesVisitor.h +++ b/dbms/src/Interpreters/ExternalTablesVisitor.h @@ -32,8 +32,8 @@ private: static void visit(const ASTIdentifier & node, ASTPtr &, Data & data) { if (auto opt_name = IdentifierSemantic::getTableName(node)) - if (StoragePtr external_storage = data.context.tryGetExternalTable(*opt_name)) - data.external_tables[*opt_name] = external_storage; + if (auto resolved_id = data.context.tryResolveStorageID(StorageID("", *opt_name), Context::ResolveExternal)) + data.external_tables[*opt_name] = DatabaseCatalog::instance().getTable(resolved_id, data.context); } }; diff --git a/dbms/src/Interpreters/InterpreterDropQuery.cpp b/dbms/src/Interpreters/InterpreterDropQuery.cpp index af32089c7a9..4f2affb07f1 100644 --- a/dbms/src/Interpreters/InterpreterDropQuery.cpp +++ b/dbms/src/Interpreters/InterpreterDropQuery.cpp @@ -209,9 +209,10 @@ BlockIO InterpreterDropQuery::executeToTemporaryTable(const String & table_name, else { auto & context_handle = context.hasSessionContext() ? context.getSessionContext() : context; - StoragePtr table = context_handle.tryGetExternalTable(table_name); - if (table) + auto resolved_id = context_handle.tryResolveStorageID(StorageID("", table_name), Context::ResolveExternal); + if (resolved_id) { + StoragePtr table = DatabaseCatalog::instance().getTable(resolved_id, context); if (kind == ASTDropQuery::Kind::Truncate) { /// If table was already dropped by anyone, an exception will be thrown diff --git a/dbms/src/Storages/StorageID.h b/dbms/src/Storages/StorageID.h index 2cc19818de5..b93a62bd6ba 100644 --- a/dbms/src/Storages/StorageID.h +++ b/dbms/src/Storages/StorageID.h @@ -53,6 +53,11 @@ struct StorageID + (hasUUID() ? " (UUID " + toString(uuid) + ")" : ""); } + explicit operator bool () const + { + return !empty(); + } + bool empty() const { return table_name.empty() && !hasUUID();