diff --git a/src/Interpreters/DatabaseCatalog.h b/src/Interpreters/DatabaseCatalog.h index c05a9a17a056..77e1d636c3be 100644 --- a/src/Interpreters/DatabaseCatalog.h +++ b/src/Interpreters/DatabaseCatalog.h @@ -156,8 +156,10 @@ class DatabaseCatalog : boost::noncopyable, WithMutableContext /// In general case Datalake catalog is a some remote service which contains iceberg/delta tables. /// Sometimes this service charges money for requests. With this flag we explicitly protect ourself /// to not accidentally query external non-free service for some trivial things like - /// autocompletion hints or system.tables query. We have a setting which allow to show + /// autocompletion hints or system.tables / system.columns queries. We have a setting which allow to show /// these databases everywhere, but user must explicitly specify it. + /// Note: system.databases always passes with_datalake_catalogs = true because listing a database name + /// is purely local metadata and never requires calls to an external catalog service. Databases getDatabases(GetDatabasesOptions options) const; /// Same as getDatabase(const String & database_name), but if database_name is empty, current database of local_context is used diff --git a/src/Interpreters/InterpreterShowTablesQuery.cpp b/src/Interpreters/InterpreterShowTablesQuery.cpp index 027d3731ab73..fe819ee4a6dc 100644 --- a/src/Interpreters/InterpreterShowTablesQuery.cpp +++ b/src/Interpreters/InterpreterShowTablesQuery.cpp @@ -232,19 +232,15 @@ BlockIO InterpreterShowTablesQuery::execute() } auto rewritten_query = getRewrittenQuery(); String database = getContext()->resolveDatabase(query.getFrom()); - if (query.databases || DatabaseCatalog::instance().isDatalakeCatalog(database)) - { - auto query_context = Context::createCopy(getContext()); - query_context->makeQueryContext(); - query_context->setCurrentQueryId(""); - /// HACK To always show them in explicit "SHOW TABLES" queries - query_context->setSetting("show_data_lake_catalogs_in_system_tables", true); - return executeQuery(rewritten_query, std::move(query_context), QueryFlags{ .internal = true }).second; - } - auto query_context = Context::createCopy(getContext()); query_context->makeQueryContext(); query_context->setCurrentQueryId(""); + if (DatabaseCatalog::instance().isDatalakeCatalog(database)) + { + /// HACK: force the setting so that system.tables includes tables from the requested data lake catalog. + /// system.databases already shows all catalogs unconditionally, so no override is needed for SHOW DATABASES. + query_context->setSetting("show_data_lake_catalogs_in_system_tables", true); + } return executeQuery(rewritten_query, std::move(query_context), QueryFlags{ .internal = true }).second; } diff --git a/src/Storages/System/StorageSystemDatabases.cpp b/src/Storages/System/StorageSystemDatabases.cpp index 3d1c262feed0..fee7227aed44 100644 --- a/src/Storages/System/StorageSystemDatabases.cpp +++ b/src/Storages/System/StorageSystemDatabases.cpp @@ -12,7 +12,6 @@ #include #include #include -#include namespace DB @@ -23,10 +22,6 @@ namespace ErrorCodes extern const int UNKNOWN_DATABASE; } -namespace Setting -{ - extern const SettingsBool show_data_lake_catalogs_in_system_tables; -} ColumnsDescription StorageSystemDatabases::getColumnsDescription() { @@ -124,8 +119,10 @@ void StorageSystemDatabases::fillData(MutableColumns & res_columns, ContextPtr c { const auto access = context->getAccess(); const bool need_to_check_access_for_databases = !access->isGranted(AccessType::SHOW_DATABASES); - const auto & settings = context->getSettingsRef(); - const auto databases = DatabaseCatalog::instance().getDatabases(GetDatabasesOptions{.with_datalake_catalogs = settings[Setting::show_data_lake_catalogs_in_system_tables]}); + /// Data lake catalog databases are always shown in system.databases regardless of show_data_lake_catalogs_in_system_tables. + /// Listing a database name is purely local metadata and never requires expensive calls to an external catalog service. + /// The setting only guards operations like system.tables / system.columns that enumerate a catalog's contents. + const auto databases = DatabaseCatalog::instance().getDatabases(GetDatabasesOptions{.with_datalake_catalogs = true}); ColumnPtr filtered_databases_column = getFilteredDatabases(databases, predicate, context); for (size_t i = 0; i < filtered_databases_column->size(); ++i) diff --git a/tests/integration/test_database_glue/test.py b/tests/integration/test_database_glue/test.py index db0edf7bdef9..9aa2c75ab59e 100644 --- a/tests/integration/test_database_glue/test.py +++ b/tests/integration/test_database_glue/test.py @@ -657,9 +657,10 @@ def test_system_tables(started_cluster): assert int(node.query(f"SELECT count() FROM system.iceberg_history WHERE database = '{CATALOG_NAME}' and table ilike '%{root_namespace}%' SETTINGS show_data_lake_catalogs_in_system_tables = true").strip()) == 4 assert int(node.query(f"SELECT count() FROM system.iceberg_history WHERE database = '{CATALOG_NAME}' and table ilike '%{root_namespace}%' SETTINGS show_data_lake_catalogs_in_system_tables = false").strip()) == 4 - # system.databases + # system.databases always shows data lake catalog databases regardless of show_data_lake_catalogs_in_system_tables, + # because listing a database name is purely local metadata and never requires calls to an external catalog service. assert int(node.query(f"SELECT count() FROM system.databases WHERE name = '{CATALOG_NAME}' SETTINGS show_data_lake_catalogs_in_system_tables = true").strip()) == 1 - assert int(node.query(f"SELECT count() FROM system.databases WHERE name = '{CATALOG_NAME}'").strip()) == 0 + assert int(node.query(f"SELECT count() FROM system.databases WHERE name = '{CATALOG_NAME}' SETTINGS show_data_lake_catalogs_in_system_tables = false").strip()) == 1 # system.columns assert int(node.query(f"SELECT count() FROM system.columns WHERE database = '{CATALOG_NAME}' and table ilike '%{root_namespace}%' SETTINGS show_data_lake_catalogs_in_system_tables = true").strip()) == 24