diff --git a/src/Access/ContextAccess.cpp b/src/Access/ContextAccess.cpp index a74f6e8e7a0..a55d0768a7e 100644 --- a/src/Access/ContextAccess.cpp +++ b/src/Access/ContextAccess.cpp @@ -392,9 +392,12 @@ bool ContextAccess::checkAccessImpl2(const AccessFlags & flags, const Args &... if (!getUser()) return access_denied("User has been dropped", ErrorCodes::UNKNOWN_USER); - /// If the current user was allowed to create a temporary table - /// then he is allowed to do with it whatever he wants. - if ((sizeof...(args) >= 2) && (getDatabase(args...) == DatabaseCatalog::TEMPORARY_DATABASE)) + /// Access to temporary tables is controlled in an unusual way, not like normal tables. + /// Creating of temporary tables is controlled by AccessType::CREATE_TEMPORARY_TABLES grant, + /// and other grants are considered as always given. + /// The DatabaseCatalog class won't resolve StorageID for temporary tables + /// which shouldn't be accessed. + if (getDatabase(args...) == DatabaseCatalog::TEMPORARY_DATABASE) return access_granted(); auto acs = getAccessRightsWithImplicit(); diff --git a/src/Storages/System/StorageSystemColumns.cpp b/src/Storages/System/StorageSystemColumns.cpp index b0371fa12de..5bc439f3bc0 100644 --- a/src/Storages/System/StorageSystemColumns.cpp +++ b/src/Storages/System/StorageSystemColumns.cpp @@ -272,17 +272,20 @@ Pipe StorageSystemColumns::read( Pipes pipes; { - Databases databases = DatabaseCatalog::instance().getDatabases(); - /// Add `database` column. MutableColumnPtr database_column_mut = ColumnString::create(); - for (const auto & database : databases) + + const auto databases = DatabaseCatalog::instance().getDatabases(); + for (const auto & [database_name, database] : databases) { + if (database_name == DatabaseCatalog::TEMPORARY_DATABASE) + continue; /// We don't want to show the internal database for temporary tables in system.columns + /// We are skipping "Lazy" database because we cannot afford initialization of all its tables. /// This should be documented. - if (database.second->getEngineName() != "Lazy") - database_column_mut->insert(database.first); + if (database->getEngineName() != "Lazy") + database_column_mut->insert(database_name); } block_to_filter.insert(ColumnWithTypeAndName(std::move(database_column_mut), std::make_shared(), "database")); @@ -305,7 +308,7 @@ Pipe StorageSystemColumns::read( for (size_t i = 0; i < rows; ++i) { const std::string database_name = (*database_column)[i].get(); - const DatabasePtr database = databases.at(database_name); + const DatabasePtr & database = databases.at(database_name); offsets[i] = i ? offsets[i - 1] : 0; for (auto iterator = database->getTablesIterator(context); iterator->isValid(); iterator->next()) diff --git a/src/Storages/System/StorageSystemDatabases.cpp b/src/Storages/System/StorageSystemDatabases.cpp index 2a89bf34186..88ac987014d 100644 --- a/src/Storages/System/StorageSystemDatabases.cpp +++ b/src/Storages/System/StorageSystemDatabases.cpp @@ -25,17 +25,20 @@ void StorageSystemDatabases::fillData(MutableColumns & res_columns, const Contex const auto access = context.getAccess(); const bool check_access_for_databases = !access->isGranted(AccessType::SHOW_DATABASES); - auto databases = DatabaseCatalog::instance().getDatabases(); - for (const auto & database : databases) + const auto databases = DatabaseCatalog::instance().getDatabases(); + for (const auto & [database_name, database] : databases) { - if (check_access_for_databases && !access->isGranted(AccessType::SHOW_DATABASES, database.first)) + if (check_access_for_databases && !access->isGranted(AccessType::SHOW_DATABASES, database_name)) continue; - res_columns[0]->insert(database.first); - res_columns[1]->insert(database.second->getEngineName()); - res_columns[2]->insert(context.getPath() + database.second->getDataPath()); - res_columns[3]->insert(database.second->getMetadataPath()); - res_columns[4]->insert(database.second->getUUID()); + if (database_name == DatabaseCatalog::TEMPORARY_DATABASE) + continue; /// We don't want to show the internal database for temporary tables in system.databases + + res_columns[0]->insert(database_name); + res_columns[1]->insert(database->getEngineName()); + res_columns[2]->insert(context.getPath() + database->getDataPath()); + res_columns[3]->insert(database->getMetadataPath()); + res_columns[4]->insert(database->getUUID()); } } diff --git a/src/Storages/System/StorageSystemTables.cpp b/src/Storages/System/StorageSystemTables.cpp index c9b3223ee38..9e3001c7dd6 100644 --- a/src/Storages/System/StorageSystemTables.cpp +++ b/src/Storages/System/StorageSystemTables.cpp @@ -65,8 +65,15 @@ StorageSystemTables::StorageSystemTables(const StorageID & table_id_) static ColumnPtr getFilteredDatabases(const ASTPtr & query, const Context & context) { MutableColumnPtr column = ColumnString::create(); - for (const auto & db : DatabaseCatalog::instance().getDatabases()) - column->insert(db.first); + + const auto databases = DatabaseCatalog::instance().getDatabases(); + for (const auto & database_name : databases | boost::adaptors::map_keys) + { + if (database_name == DatabaseCatalog::TEMPORARY_DATABASE) + continue; /// We don't want to show the internal database for temporary tables in system.tables + + column->insert(database_name); + } Block block { ColumnWithTypeAndName(std::move(column), std::make_shared(), "database") }; VirtualColumnUtils::filterBlockWithQuery(query, block, context); diff --git a/tests/queries/0_stateless/01098_temporary_and_external_tables.reference b/tests/queries/0_stateless/01098_temporary_and_external_tables.reference index c3e51f64b2d..1ddb10581a8 100644 --- a/tests/queries/0_stateless/01098_temporary_and_external_tables.reference +++ b/tests/queries/0_stateless/01098_temporary_and_external_tables.reference @@ -1,2 +1,9 @@ +CREATE TEMPORARY TABLE tmp_table\n(\n `n` UInt64\n)\nENGINE = Memory AS\nSELECT number AS n\nFROM numbers(42) +CREATE TEMPORARY TABLE tmp_table\n(\n `n` UInt64\n)\nENGINE = Memory AS\nSELECT number AS n\nFROM numbers(42) +CREATE TEMPORARY TABLE tmp_table\n(\n `n` UInt64\n)\nENGINE = Memory AS\nSELECT number AS n\nFROM numbers(42) +42 +OK OK 3 9 +0 +1 diff --git a/tests/queries/0_stateless/01098_temporary_and_external_tables.sh b/tests/queries/0_stateless/01098_temporary_and_external_tables.sh index 67f57567298..54834107728 100755 --- a/tests/queries/0_stateless/01098_temporary_and_external_tables.sh +++ b/tests/queries/0_stateless/01098_temporary_and_external_tables.sh @@ -6,20 +6,23 @@ CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) url_without_session="https://${CLICKHOUSE_HOST}:${CLICKHOUSE_PORT_HTTPS}/?" url="${url_without_session}session_id=test_01098" -${CLICKHOUSE_CURL} -m 30 -sSk "$url" --data "CREATE TEMPORARY TABLE tmp_table AS SELECT number AS n FROM numbers(42)" > /dev/null; +${CLICKHOUSE_CURL} -m 30 -sSk "$url" --data "DROP TEMPORARY TABLE IF EXISTS tmp_table" +${CLICKHOUSE_CURL} -m 30 -sSk "$url" --data "CREATE TEMPORARY TABLE tmp_table AS SELECT number AS n FROM numbers(42)" -name_expr="'\`' || database || '\`.\`' || name || '\`'" -full_tmp_name=$(echo "SELECT $name_expr FROM system.tables WHERE database='_temporary_and_external_tables' AND create_table_query LIKE '%tmp_table%'" | ${CLICKHOUSE_CURL} -m 30 -sSgk "$url" -d @-) +id=$(echo "SELECT uuid FROM system.tables WHERE name='tmp_table' AND is_temporary" | ${CLICKHOUSE_CURL} -m 31 -sSgk "$url" -d @-) +internal_table_name="_temporary_and_external_tables.\`_tmp_$id\`" -echo "SELECT * FROM $full_tmp_name" | ${CLICKHOUSE_CURL} -m 60 -sSgk "$url" -d @- | grep -F "Code: 291" > /dev/null && echo "OK" +${CLICKHOUSE_CURL} -m 30 -sSk "$url" --data "SHOW CREATE TEMPORARY TABLE tmp_table" +${CLICKHOUSE_CURL} -m 30 -sSk "$url" --data "SHOW CREATE TABLE $internal_table_name" +${CLICKHOUSE_CURL} -m 30 -sSk "$url_without_session" --data "SHOW CREATE TABLE $internal_table_name" -echo -ne '0\n1\n' | ${CLICKHOUSE_CURL} -m 30 -sSkF 'file=@-' "$url&file_format=CSV&file_types=UInt64&query=SELECT+sum((number+GLOBAL+IN+(SELECT+number+AS+n+FROM+remote('127.0.0.2',+numbers(5))+WHERE+n+GLOBAL+IN+(SELECT+*+FROM+tmp_table)+AND+n+GLOBAL+NOT+IN+(SELECT+*+FROM+file)+))+AS+res),+sum(number*res)+FROM+remote('127.0.0.2',+numbers(10))"; +echo "SELECT COUNT() FROM tmp_table" | ${CLICKHOUSE_CURL} -m 60 -sSgk "$url" -d @- +echo "SELECT COUNT() FROM $internal_table_name" | ${CLICKHOUSE_CURL} -m 60 -sSgk "$url" -d @- | grep -F "Code: 291" > /dev/null && echo "OK" +echo "SELECT COUNT() FROM $internal_table_name" | ${CLICKHOUSE_CURL} -m 60 -sSgk "$url_without_session" -d @- | grep -F "Code: 291" > /dev/null && echo "OK" + +echo -ne '0\n1\n' | ${CLICKHOUSE_CURL} -m 30 -sSkF 'file=@-' "$url&file_format=CSV&file_types=UInt64&query=SELECT+sum((number+GLOBAL+IN+(SELECT+number+AS+n+FROM+remote('127.0.0.2',+numbers(5))+WHERE+n+GLOBAL+IN+(SELECT+*+FROM+tmp_table)+AND+n+GLOBAL+NOT+IN+(SELECT+*+FROM+file)+))+AS+res),+sum(number*res)+FROM+remote('127.0.0.2',+numbers(10))" + +echo -ne '0\n1\n' | ${CLICKHOUSE_CURL} -m 30 -sSkF 'file=@-' "$url&file_format=CSV&file_types=UInt64&query=SELECT+_1%2BsleepEachRow(3)+FROM+file" & -echo -ne '0\n1\n' | ${CLICKHOUSE_CURL} -m 30 -sSkF 'file=@-' "$url&file_format=CSV&file_types=UInt64&query=SELECT+sleepEachRow(3)+FROM+file" > /dev/null & -sleep 1 -full_tmp_names=$(echo "SELECT $name_expr FROM system.tables WHERE database='_temporary_and_external_tables' FORMAT TSV" | ${CLICKHOUSE_CURL} -m 30 -sSgk "$url_without_session" -d @-) -for name in $full_tmp_names -do - ${CLICKHOUSE_CURL} -m 30 -sSk "${url_without_session}query=SHOW+CREATE+TABLE+$name" 1>/dev/null 2>/dev/null -done; wait +${CLICKHOUSE_CURL} -m 30 -sSk "$url" --data "DROP TEMPORARY TABLE tmp_table"