From e98d4f4e5e0b57bc8c473ba90263e67eb1b99980 Mon Sep 17 00:00:00 2001 From: Alexander Tokmakov Date: Thu, 12 Mar 2020 15:16:16 +0300 Subject: [PATCH] remove isExternalTableExist --- dbms/src/Interpreters/Context.cpp | 16 +++++----------- dbms/src/Interpreters/Context.h | 2 -- dbms/src/Interpreters/DatabaseCatalog.cpp | 1 + dbms/src/Interpreters/DatabaseCatalog.h | 3 --- dbms/src/Interpreters/InterpreterCreateQuery.cpp | 2 +- dbms/src/Interpreters/InterpreterDropQuery.cpp | 3 +-- dbms/src/Interpreters/InterpreterExistsQuery.cpp | 2 +- dbms/src/Interpreters/InterpreterSelectQuery.cpp | 12 ++++++------ dbms/src/Interpreters/JoinedTables.cpp | 13 +++---------- dbms/src/Interpreters/JoinedTables.h | 6 ++---- .../00492_drop_temporary_table.reference | 2 ++ .../0_stateless/00492_drop_temporary_table.sql | 2 ++ 12 files changed, 24 insertions(+), 40 deletions(-) diff --git a/dbms/src/Interpreters/Context.cpp b/dbms/src/Interpreters/Context.cpp index 5e134417258..99ab19045e6 100644 --- a/dbms/src/Interpreters/Context.cpp +++ b/dbms/src/Interpreters/Context.cpp @@ -815,13 +815,6 @@ void Context::setProfile(const String & profile) } -bool Context::isExternalTableExist(const String & table_name) const -{ - assert(global_context != this); - auto lock = getLock(); - return external_tables_mapping.count(table_name); -} - const Scalars & Context::getScalars() const { return scalars; @@ -839,20 +832,21 @@ const Block & Context::getScalar(const String & name) const Tables Context::getExternalTables() const { + assert(global_context != this); auto lock = getLock(); Tables res; for (auto & table : external_tables_mapping) res[table.first] = table.second->getTable(); - if (session_context && session_context != this) + if (query_context && query_context != this) { - Tables buf = session_context->getExternalTables(); + Tables buf = query_context->getExternalTables(); res.insert(buf.begin(), buf.end()); } - else if (global_context && global_context != this) + else if (session_context && session_context != this) { - Tables buf = global_context->getExternalTables(); + Tables buf = session_context->getExternalTables(); res.insert(buf.begin(), buf.end()); } return res; diff --git a/dbms/src/Interpreters/Context.h b/dbms/src/Interpreters/Context.h index 334cd66b9c3..bb791329ef1 100644 --- a/dbms/src/Interpreters/Context.h +++ b/dbms/src/Interpreters/Context.h @@ -288,8 +288,6 @@ public: ClientInfo & getClientInfo() { return client_info; } const ClientInfo & getClientInfo() const { return client_info; } - bool isExternalTableExist(const String & table_name) const; - enum StorageNamespace { ResolveGlobal = 1u, /// Database name must be specified diff --git a/dbms/src/Interpreters/DatabaseCatalog.cpp b/dbms/src/Interpreters/DatabaseCatalog.cpp index f99d14b497b..0157b58f265 100644 --- a/dbms/src/Interpreters/DatabaseCatalog.cpp +++ b/dbms/src/Interpreters/DatabaseCatalog.cpp @@ -19,6 +19,7 @@ namespace ErrorCodes extern const int DATABASE_ALREADY_EXISTS; extern const int DATABASE_NOT_EMPTY; extern const int DATABASE_ACCESS_DENIED; + extern const int LOGICAL_ERROR; } TemporaryTableHolder::TemporaryTableHolder(const Context & context_, diff --git a/dbms/src/Interpreters/DatabaseCatalog.h b/dbms/src/Interpreters/DatabaseCatalog.h index 3179147c218..caa4cfa5273 100644 --- a/dbms/src/Interpreters/DatabaseCatalog.h +++ b/dbms/src/Interpreters/DatabaseCatalog.h @@ -92,9 +92,6 @@ public: static DatabaseCatalog & instance(); - //DatabaseCatalog(/*Context & global_context_, String default_database_*/) {} - //: global_context(global_context_)/*, default_database(std::move(default_database_))*/ {} - void loadDatabases(); void shutdown(); diff --git a/dbms/src/Interpreters/InterpreterCreateQuery.cpp b/dbms/src/Interpreters/InterpreterCreateQuery.cpp index 8f39fdc8715..f15796688e1 100644 --- a/dbms/src/Interpreters/InterpreterCreateQuery.cpp +++ b/dbms/src/Interpreters/InterpreterCreateQuery.cpp @@ -608,7 +608,7 @@ bool InterpreterCreateQuery::doCreateTable(const ASTCreateQuery & create, } else { - if (context.isExternalTableExist(table_name) && create.if_not_exists) + if (context.tryResolveStorageID({"", table_name}, Context::ResolveExternal) && create.if_not_exists) return false; auto temporary_table = TemporaryTableHolder(context, properties.columns, query_ptr); diff --git a/dbms/src/Interpreters/InterpreterDropQuery.cpp b/dbms/src/Interpreters/InterpreterDropQuery.cpp index 6b1f483906a..261435b8fe7 100644 --- a/dbms/src/Interpreters/InterpreterDropQuery.cpp +++ b/dbms/src/Interpreters/InterpreterDropQuery.cpp @@ -58,8 +58,7 @@ BlockIO InterpreterDropQuery::executeToTable( { if (is_temporary || database_name_.empty()) { - auto & session_context = context.hasSessionContext() ? context.getSessionContext() : context; - if (session_context.isExternalTableExist(table_name)) + if (context.tryResolveStorageID({"", table_name}, Context::ResolveExternal)) return executeToTemporaryTable(table_name, kind); } diff --git a/dbms/src/Interpreters/InterpreterExistsQuery.cpp b/dbms/src/Interpreters/InterpreterExistsQuery.cpp index c17d85a1ff0..f367af8416c 100644 --- a/dbms/src/Interpreters/InterpreterExistsQuery.cpp +++ b/dbms/src/Interpreters/InterpreterExistsQuery.cpp @@ -45,7 +45,7 @@ BlockInputStreamPtr InterpreterExistsQuery::executeImpl() if (exists_query->temporary) { context.checkAccess(AccessType::EXISTS, "", exists_query->table); - result = context.isExternalTableExist(exists_query->table); + result = !context.tryResolveStorageID({"", exists_query->table}, Context::ResolveExternal).empty(); } else { diff --git a/dbms/src/Interpreters/InterpreterSelectQuery.cpp b/dbms/src/Interpreters/InterpreterSelectQuery.cpp index e368240291e..7ae19912e40 100644 --- a/dbms/src/Interpreters/InterpreterSelectQuery.cpp +++ b/dbms/src/Interpreters/InterpreterSelectQuery.cpp @@ -321,7 +321,7 @@ InterpreterSelectQuery::InterpreterSelectQuery( /// Save the new temporary tables in the query context for (const auto & it : query_analyzer->getExternalTables()) - if (!context->isExternalTableExist(it.first)) + if (!context->tryResolveStorageID({"", it.first}, Context::ResolveExternal)) context->addExternalTable(it.first, std::move(*it.second)); } @@ -402,14 +402,14 @@ InterpreterSelectQuery::InterpreterSelectQuery( if (query.prewhere() && !query.where()) analysis_result.prewhere_info->need_filter = true; - const String & database_name = joined_tables.leftTableDatabase(); - const String & table_name = joined_tables.leftTableName(); + const StorageID & left_table_id = joined_tables.leftTableID(); - if (!table_name.empty() && !database_name.empty() /* always allow access to temporary tables */) - context->checkAccess(AccessType::SELECT, database_name, table_name, required_columns); + if (left_table_id) + context->checkAccess(AccessType::SELECT, left_table_id.getDatabaseName(), left_table_id.getTableName(), required_columns); /// Remove limits for some tables in the `system` database. - if (database_name == "system" && ((table_name == "quotas") || (table_name == "quota_usage") || (table_name == "one"))) + if (left_table_id.database_name == "system" && + ((left_table_id.table_name == "quotas") || (left_table_id.table_name == "quota_usage") || (left_table_id.table_name == "one"))) { options.ignore_quota = true; options.ignore_limits = true; diff --git a/dbms/src/Interpreters/JoinedTables.cpp b/dbms/src/Interpreters/JoinedTables.cpp index 9e581f323c5..48b1f8732f1 100644 --- a/dbms/src/Interpreters/JoinedTables.cpp +++ b/dbms/src/Interpreters/JoinedTables.cpp @@ -68,24 +68,18 @@ StoragePtr JoinedTables::getLeftTableStorage() if (left_db_and_table) { - database_name = left_db_and_table->database; - table_name = left_db_and_table->table; - - /// If the database is not specified - use the current database. - if (database_name.empty() && !context.isExternalTableExist(table_name)) - database_name = context.getCurrentDatabase(); + table_id = context.resolveStorageID(StorageID(left_db_and_table->database, left_db_and_table->table)); } else /// If the table is not specified - use the table `system.one`. { - database_name = "system"; - table_name = "one"; + table_id = StorageID("system", "one"); } if (auto view_source = context.getViewSource()) { auto & storage_values = static_cast(*view_source); auto tmp_table_id = storage_values.getStorageID(); - if (tmp_table_id.database_name == database_name && tmp_table_id.table_name == table_name) + if (tmp_table_id.database_name == table_id.database_name && tmp_table_id.table_name == table_id.table_name) { /// Read from view source. return context.getViewSource(); @@ -93,7 +87,6 @@ StoragePtr JoinedTables::getLeftTableStorage() } /// Read from table. Even without table expression (implicit SELECT ... FROM system.one). - auto table_id = context.resolveStorageID({database_name, table_name}); return DatabaseCatalog::instance().getTable(table_id); } diff --git a/dbms/src/Interpreters/JoinedTables.h b/dbms/src/Interpreters/JoinedTables.h index c9d24562a26..f1940366ef5 100644 --- a/dbms/src/Interpreters/JoinedTables.h +++ b/dbms/src/Interpreters/JoinedTables.h @@ -35,8 +35,7 @@ public: bool isLeftTableFunction() const; size_t tablesCount() const { return table_expressions.size(); } - const String & leftTableDatabase() const { return database_name; } - const String & leftTableName() const { return table_name; } + const StorageID & leftTableID() const { return table_id; } std::unique_ptr makeLeftTableSubquery(const SelectQueryOptions & select_options); @@ -50,8 +49,7 @@ private: std::optional left_db_and_table; /// left_db_and_table or 'system.one' - String database_name; - String table_name; + StorageID table_id = StorageID::createEmpty(); }; } diff --git a/dbms/tests/queries/0_stateless/00492_drop_temporary_table.reference b/dbms/tests/queries/0_stateless/00492_drop_temporary_table.reference index 573541ac970..7938dcdde86 100644 --- a/dbms/tests/queries/0_stateless/00492_drop_temporary_table.reference +++ b/dbms/tests/queries/0_stateless/00492_drop_temporary_table.reference @@ -1 +1,3 @@ 0 +1 +0 diff --git a/dbms/tests/queries/0_stateless/00492_drop_temporary_table.sql b/dbms/tests/queries/0_stateless/00492_drop_temporary_table.sql index 06ec5ea9805..e990f502d4d 100644 --- a/dbms/tests/queries/0_stateless/00492_drop_temporary_table.sql +++ b/dbms/tests/queries/0_stateless/00492_drop_temporary_table.sql @@ -3,7 +3,9 @@ CREATE TEMPORARY TABLE temp_tab (number UInt64); INSERT INTO temp_tab SELECT number FROM system.numbers LIMIT 1; SELECT number FROM temp_tab; SET send_logs_level = 'none'; +EXISTS temp_tab; DROP TABLE temp_tab; +EXISTS temp_tab; SET send_logs_level = 'warning'; CREATE TEMPORARY TABLE temp_tab (number UInt64); SELECT number FROM temp_tab;