Fix access rights required for the merge() table function.

src/Storages/StorageMerge.cpp

@@ -72,11 +72,27 @@ StorageMerge::StorageMerge(
     const StorageID & table_id_,
     const ColumnsDescription & columns_,
     const String & source_database_,
-    const String & table_name_regexp_,
+    const Strings & source_tables_,
     const Context & context_)
     : IStorage(table_id_)
     , source_database(source_database_)
-    , table_name_regexp(table_name_regexp_)
+    , source_tables(std::in_place, source_tables_.begin(), source_tables_.end())
+    , global_context(context_.getGlobalContext())
+{
+    StorageInMemoryMetadata storage_metadata;
+    storage_metadata.setColumns(columns_);
+    setInMemoryMetadata(storage_metadata);
+}
+
+StorageMerge::StorageMerge(
+    const StorageID & table_id_,
+    const ColumnsDescription & columns_,
+    const String & source_database_,
+    const String & source_table_regexp_,
+    const Context & context_)
+    : IStorage(table_id_)
+    , source_database(source_database_)
+    , source_table_regexp(source_table_regexp_)
     , global_context(context_.getGlobalContext())
 {
     StorageInMemoryMetadata storage_metadata;
@@ -439,8 +455,17 @@ DatabaseTablesIteratorPtr StorageMerge::getDatabaseIterator(const Context & cont
         e.addMessage("while getting table iterator of Merge table. Maybe caused by two Merge tables that will endlessly try to read each other's data");
         throw;
     }
+
     auto database = DatabaseCatalog::instance().getDatabase(source_database);
-    auto table_name_match = [this](const String & table_name_) { return table_name_regexp.match(table_name_); };
+
+    auto table_name_match = [this](const String & table_name_) -> bool
+    {
+        if (source_tables)
+            return source_tables->count(table_name_);
+        else
+            return source_table_regexp->match(table_name_);
+    };
+
     return database->getTablesIterator(context, table_name_match);
 }
 

src/Storages/StorageMerge.h

@@ -48,7 +48,8 @@ public:
 
 private:
     String source_database;
-    OptimizedRegularExpression table_name_regexp;
+    std::optional<std::unordered_set<String>> source_tables;
+    std::optional<OptimizedRegularExpression> source_table_regexp;
     const Context & global_context;
 
     using StorageWithLockAndName = std::tuple<StoragePtr, TableLockHolder, String>;
@@ -72,7 +73,14 @@ protected:
         const StorageID & table_id_,
         const ColumnsDescription & columns_,
         const String & source_database_,
-        const String & table_name_regexp_,
+        const Strings & source_tables_,
+        const Context & context_);
+
+    StorageMerge(
+        const StorageID & table_id_,
+        const ColumnsDescription & columns_,
+        const String & source_database_,
+        const String & source_table_regexp_,
         const Context & context_);
 
     Pipe createSources(

src/TableFunctions/TableFunctionMerge.cpp

@@ -6,6 +6,7 @@
 #include <TableFunctions/ITableFunction.h>
 #include <Interpreters/evaluateConstantExpression.h>
 #include <Interpreters/Context.h>
+#include <Access/ContextAccess.h>
 #include <TableFunctions/TableFunctionMerge.h>
 #include <TableFunctions/TableFunctionFactory.h>
 #include <TableFunctions/registerTableFunctions.h>
@@ -14,37 +15,24 @@
 namespace DB
 {
 
-
 namespace ErrorCodes
 {
     extern const int NUMBER_OF_ARGUMENTS_DOESNT_MATCH;
     extern const int UNKNOWN_TABLE;
 }
 
-
-static NamesAndTypesList chooseColumns(const String & source_database, const String & table_name_regexp_, const Context & context)
+namespace
 {
-    OptimizedRegularExpression table_name_regexp(table_name_regexp_);
-    auto table_name_match = [&](const String & table_name) { return table_name_regexp.match(table_name); };
-
-    StoragePtr any_table;
-
+    [[noreturn]] void throwNoTablesMatchRegexp(const String & source_database, const String & source_table_regexp)
     {
-        auto database = DatabaseCatalog::instance().getDatabase(source_database);
-        auto iterator = database->getTablesIterator(context, table_name_match);
-
-        if (iterator->isValid())
-            if (const auto & table = iterator->table())
-                any_table = table;
+        throw Exception(
+            "Error while executing table function merge. In database " + source_database
+                + " no one matches regular expression: " + source_table_regexp,
+            ErrorCodes::UNKNOWN_TABLE);
     }
-
-    if (!any_table)
-        throw Exception("Error while executing table function merge. In database " + source_database + " no one matches regular expression: "
-            + table_name_regexp_, ErrorCodes::UNKNOWN_TABLE);
-
-    return any_table->getInMemoryMetadataPtr()->getColumns().getAllPhysical();
 }
 
+
 void TableFunctionMerge::parseArguments(const ASTPtr & ast_function, const Context & context)
 {
     ASTs & args_func = ast_function->children;
@@ -65,21 +53,64 @@ void TableFunctionMerge::parseArguments(const ASTPtr & ast_function, const Conte
     args[1] = evaluateConstantExpressionAsLiteral(args[1], context);
 
     source_database = args[0]->as<ASTLiteral &>().value.safeGet<String>();
-    table_name_regexp = args[1]->as<ASTLiteral &>().value.safeGet<String>();
+    source_table_regexp = args[1]->as<ASTLiteral &>().value.safeGet<String>();
 }
 
+
+const Strings & TableFunctionMerge::getSourceTables(const Context & context) const
+{
+    if (source_tables)
+        return *source_tables;
+
+    auto database = DatabaseCatalog::instance().getDatabase(source_database);
+
+    OptimizedRegularExpression re(source_table_regexp);
+    auto table_name_match = [&](const String & table_name_) { return re.match(table_name_); };
+
+    auto access = context.getAccess();
+    bool granted_show_on_all_tables = access->isGranted(AccessType::SHOW_TABLES, source_database);
+    bool granted_select_on_all_tables = access->isGranted(AccessType::SELECT, source_database);
+
+    source_tables.emplace();
+    for (auto it = database->getTablesIterator(context, table_name_match); it->isValid(); it->next())
+    {
+        if (!it->table())
+            continue;
+        bool granted_show = granted_show_on_all_tables || access->isGranted(AccessType::SHOW_TABLES, source_database, it->name());
+        if (!granted_show)
+            continue;
+        if (!granted_select_on_all_tables)
+            access->checkAccess(AccessType::SELECT, source_database, it->name());
+        source_tables->emplace_back(it->name());
+    }
+
+    if (source_tables->empty())
+        throwNoTablesMatchRegexp(source_database, source_table_regexp);
+
+    return *source_tables;
+}
+
+
 ColumnsDescription TableFunctionMerge::getActualTableStructure(const Context & context) const
 {
-    return ColumnsDescription{chooseColumns(source_database, table_name_regexp, context)};
+    for (const auto & table_name : getSourceTables(context))
+    {
+        auto storage = DatabaseCatalog::instance().tryGetTable(StorageID{source_database, table_name}, context);
+        if (storage)
+            return ColumnsDescription{storage->getInMemoryMetadataPtr()->getColumns().getAllPhysical()};
+    }
+
+    throwNoTablesMatchRegexp(source_database, source_table_regexp);
 }
 
+
 StoragePtr TableFunctionMerge::executeImpl(const ASTPtr & /*ast_function*/, const Context & context, const std::string & table_name, ColumnsDescription /*cached_columns*/) const
 {
     auto res = StorageMerge::create(
         StorageID(getDatabaseName(), table_name),
         getActualTableStructure(context),
         source_database,
-        table_name_regexp,
+        getSourceTables(context),
         context);
 
     res->startup();

src/TableFunctions/TableFunctionMerge.h

@@ -19,11 +19,13 @@ private:
     StoragePtr executeImpl(const ASTPtr & ast_function, const Context & context, const std::string & table_name, ColumnsDescription cached_columns) const override;
     const char * getStorageTypeName() const override { return "Merge"; }
 
+    const Strings & getSourceTables(const Context & context) const;
     ColumnsDescription getActualTableStructure(const Context & context) const override;
     void parseArguments(const ASTPtr & ast_function, const Context & context) override;
 
     String source_database;
-    String table_name_regexp;
+    String source_table_regexp;
+    mutable std::optional<Strings> source_tables;
 };
 
 

tests/integration/test_table_functions_access_rights/test.py

@@ -0,0 +1,55 @@
+import pytest
+from helpers.cluster import ClickHouseCluster
+from helpers.test_tools import TSV
+
+cluster = ClickHouseCluster(__file__)
+instance = cluster.add_instance('instance')
+
+
+@pytest.fixture(scope="module", autouse=True)
+def started_cluster():
+    try:
+        cluster.start()
+
+        instance.query("CREATE TABLE table1(x UInt32) ENGINE = MergeTree ORDER BY tuple()")
+        instance.query("CREATE TABLE table2(x UInt32) ENGINE = MergeTree ORDER BY tuple()")
+        instance.query("INSERT INTO table1 VALUES (1)")
+        instance.query("INSERT INTO table2 VALUES (2)")
+
+        yield cluster
+
+    finally:
+        cluster.shutdown()
+
+
+@pytest.fixture(autouse=True)
+def cleanup_after_test():
+    try:
+        yield
+    finally:
+        instance.query("DROP USER IF EXISTS A")
+
+
+def test_merge():
+    select_query = "SELECT * FROM merge('default', 'table[0-9]+') ORDER BY x"
+    assert instance.query(select_query) == "1\n2\n"
+
+    instance.query("CREATE USER A")
+    assert "it's necessary to have grant CREATE TEMPORARY TABLE ON *.*" in instance.query_and_get_error(select_query, user = 'A')
+
+    instance.query("GRANT CREATE TEMPORARY TABLE ON *.* TO A")
+    assert "no one matches regular expression" in instance.query_and_get_error(select_query, user = 'A')
+    
+    instance.query("GRANT SELECT ON default.table1 TO A")
+    assert instance.query(select_query, user = 'A') == "1\n"
+
+    instance.query("GRANT SELECT ON default.* TO A")
+    assert instance.query(select_query, user = 'A') == "1\n2\n"
+
+    instance.query("REVOKE SELECT ON default.table1 FROM A")
+    assert instance.query(select_query, user = 'A') == "2\n"
+
+    instance.query("REVOKE ALL ON default.* FROM A")
+    instance.query("GRANT SELECT ON default.table1 TO A")
+    instance.query("GRANT INSERT ON default.table2 TO A")
+    assert "it's necessary to have grant SELECT ON default.table2" in instance.query_and_get_error(select_query, user = 'A')

tests/testflows/rbac/tests/privileges/create/create_table.py

@@ -706,6 +706,9 @@ def create_as_merge(self, node=None):
                 with When("I grant CREATE TABLE privilege to a user"):
                     node.query(f"GRANT CREATE TABLE ON {table_name} TO {user_name}")
 
+                with And("I grant SELECT privilege to a user to allow executing the table function merge()"):
+                    node.query(f"GRANT SELECT ON {source_table_name} TO {user_name}")
+
                 with Then("I try to create a table as another table"):
                     node.query(f"CREATE TABLE {table_name} AS merge(default,'{source_table_name}')", settings = [("user", f"{user_name}")])