remove tryGetExternalTable

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;
+    auto temporary_id = StorageID::createEmpty();
-    readStringBinary(name, *in);
+    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);
             }

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<Exception> 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> * exception) const
 {
     if (storage_id.uuid != UUIDHelpers::Nil)
         return storage_id;
 
-    bool look_for_external_table = where & StorageNamespace::External;
+    if (storage_id.empty())
-    bool in_current_database = where & StorageNamespace::CurrentDatabase;
+    {
-    bool in_specified_database = where & StorageNamespace::Global;
+        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();
 }
 
 

dbms/src/Interpreters/Context.h

@@ -287,22 +287,22 @@ public:
 
     enum StorageNamespace
     {
-         Global = 1u,                           /// Database name must be specified
+         ResolveGlobal = 1u,                                           /// Database name must be specified
-         CurrentDatabase = 2u,                  /// Use current database
+         ResolveCurrentDatabase = 2u,                                  /// Use current database
-         Ordinary = Global | CurrentDatabase,   /// If database name is not specified, use current database
+         ResolveOrdinary = ResolveGlobal | ResolveCurrentDatabase,     /// If database name is not specified, use current database
-         External = 4u,                         /// Try get external table
+         ResolveExternal = 4u,                                         /// Try get external table
-         All = External | Ordinary              /// If database name is not specified, 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.
+                                                                       ///    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 resolveStorageID(StorageID storage_id, StorageNamespace where = StorageNamespace::ResolveAll) const;
-    StorageID resolveStorageIDUnlocked(StorageID storage_id, StorageNamespace where = StorageNamespace::All) const;
+    StorageID tryResolveStorageID(StorageID storage_id, StorageNamespace where = StorageNamespace::ResolveAll) const;
+    StorageID resolveStorageIDImpl(StorageID storage_id, StorageNamespace where, std::optional<Exception> * 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;

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<DDLGuard> DatabaseCatalog::getDDLGuard(const String & database, const String & table)

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> * exception) const;
+    StoragePtr getTable(const StorageID & table_id, const Context & local_context, std::optional<Exception> * exception = nullptr) const;
 
 
     void addDependency(const StorageID & from, const StorageID & where);

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))
+            if (auto resolved_id = data.context.tryResolveStorageID(StorageID("", *opt_name), Context::ResolveExternal))
-                data.external_tables[*opt_name] = external_storage;
+                data.external_tables[*opt_name] = DatabaseCatalog::instance().getTable(resolved_id, data.context);
     }
 };
 

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);
+        auto resolved_id = context_handle.tryResolveStorageID(StorageID("", table_name), Context::ResolveExternal);
-        if (table)
+        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

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();