Add tables with authorization information to system.metadata #25907

homar · 2025-06-02T21:01:43Z

Description

Trino supports ALTER (TABLE | FUNCTION | SCHEMA) SET AUTHORIZATION for quite some time.
However there is no way to retrieve this information. This commit fixes this by
introducing system.metadata.(tables | schemas | functions)_authorization tables.

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(x) Release notes are required, with the following suggested text:

## Section
* Introduces system.metadata.(tables | schemas | functions)_authorization tables

…25907 Trino supports ALTER (TABLE | FUNCTION | SCHEMA) SET AUTHORIZATION for quite some time. However there is no way to retrieve this information. This commit fixes this by introducing system.metadata.(tables | schemas | functions)_authorization tables.

lukasz-walkiewicz · 2025-06-04T07:38:58Z

core/trino-main/src/main/java/io/trino/metadata/Metadata.java

+    /**
+     * Returns list of schemas authorization info
+     */
+    Set<SchemaAuthorization> getAllSchemasAuthorizationInfo(Session session, QualifiedTablePrefix prefix);


nit: put schema first, then tables and functions last?

lukasz-walkiewicz · 2025-06-04T07:49:34Z

core/trino-main/src/main/java/io/trino/metadata/SystemSecurityMetadata.java

+    /**
+     * Returns all tables authorization information
+     */
+    Set<TableAuthorization> getAllTablesAuthorizationInfo(Session session, QualifiedTablePrefix prefix);


nit: I'd say All infix is redundant

lukasz-walkiewicz · 2025-06-04T08:07:27Z

testing/trino-tests/src/test/java/io/trino/security/TestingSystemSecurityMetadata.java

+                .build();
+    }
+
+    private Set<TableAuthorization> getOwnershipInfoInternal(Map<CatalogSchemaTableName, Identity> owners, QualifiedTablePrefix prefix)


nit: just combine maps instead of extracting this method?

lukasz-walkiewicz · 2025-06-04T08:14:33Z

core/trino-main/src/main/java/io/trino/metadata/MetadataListing.java

@@ -85,6 +86,20 @@ public static List<CatalogInfo> listCatalogs(Session session, Metadata metadata,
                .collect(toImmutableList());
    }

+    public static Set<CatalogSchemaName> listAllAvailableSchemas(


Is this method actually needed if all it does is delegating to listCatalogNames and listSchemas?

it only does delegation BUT i would need to reuse this code in 3 places so I decided it is a good idea to actually extract it to a separated method

lukasz-walkiewicz · 2025-06-04T08:17:28Z

testing/trino-tests/src/test/java/io/trino/security/TestAccessControl.java

+        getQueryRunner().execute(tableOwner1, "CREATE TABLE %s (id INT)".formatted(table1));
+        assertQuery(
+                "SELECT * FROM system.metadata.tables_authorization",
+                "VALUES('blackhole', 'default', '%s', 'USER', '%s')".formatted(table1, tableOwnerName1));


That's the only test using blackhole catalog instead of memory_test, why?

because I was not able to create a function in blackhole catalog so functions are tested using memory (and memory_test to check that table returns functions from different catalogs).
I can change this to also use memory

kokosing · 2025-06-04T19:42:05Z

core/trino-main/src/main/java/io/trino/metadata/SystemSecurityMetadata.java

@@ -240,4 +243,19 @@ default void validateEntityKindAndPrivileges(Session session, String entityKind,
     * to be fully qualified, i.e., if the entity is a table, the name is of size three.
     */
    void setEntityOwner(Session session, EntityKindAndName entityKindAndName, TrinoPrincipal principal);


Don't we need a generic function like this one? Also I am not a fun that in one place we call it "authorization" and in other "owner". Maybe we should rename setEntityOwner to setEntityAuthorization?

Map<String, TrinoPrincipal> getEntityAuthorizations(Session session, String entityKind); Optional<TrinoPrincipal> getEntityAuthorization(Session session, EntityKindAndName entityKindAndName);

?

Also, if we would go this path then it means we just need system.metadata.entity_authorization table.

I see your point, however I am not sure which path to follow now. Both makes sense, I think the generic one may be little more complex and there will be place in the code where finally we will map those generic types to tables + schemas + functions so I am not sure if this is worth the effort.
Your call

kokosing · 2025-06-04T19:42:58Z

core/trino-main/src/main/java/io/trino/metadata/SystemSecurityMetadata.java

@@ -240,4 +243,19 @@ default void validateEntityKindAndPrivileges(Session session, String entityKind,
     * to be fully qualified, i.e., if the entity is a table, the name is of size three.
     */
    void setEntityOwner(Session session, EntityKindAndName entityKindAndName, TrinoPrincipal principal);
+
+    /**
+     * Returns all tables authorization information


nit: This javadocs does not say more that the method name. I would drop it.

kokosing · 2025-06-04T19:45:14Z

core/trino-main/src/main/java/io/trino/metadata/SystemSecurityMetadata.java

+    /**
+     * Returns all schemas authorization information
+     */
+    Set<SchemaAuthorization> getAllSchemasAuthorizationInfo(Session session, QualifiedTablePrefix prefix);


It is odd to pass QualifiedTablePrefix as it is not a table. I mean if one passed schema_name parameter then you should return just that schema.

when schema name is passed then it is easy, but this is an optimization for situation when only catalog is known, instead of returning all schemas from all catalogs when can just get schemas for a specific catalog

…25907 Trino supports ALTER (TABLE | FUNCTION | SCHEMA) SET AUTHORIZATION for quite some time. However there is no way to retrieve this information. This commit fixes this by introducing system.metadata.(tables | schemas | functions)_authorization tables.

cla-bot bot added the cla-signed label Jun 2, 2025

homar force-pushed the homar/add_authorizations_metadata branch 2 times, most recently from a7392e0 to 2ff0516 Compare June 2, 2025 22:03

homar changed the title ~~Homar/add authorizations metadata~~ Add tables with authorization information to system.metadata Jun 3, 2025

homar force-pushed the homar/add_authorizations_metadata branch from 2ff0516 to d6e48b8 Compare June 3, 2025 10:39

homar marked this pull request as ready for review June 3, 2025 10:40

lukasz-walkiewicz reviewed Jun 4, 2025

View reviewed changes

lukasz-walkiewicz requested review from martint and kokosing June 4, 2025 08:19

kokosing reviewed Jun 4, 2025

View reviewed changes

homar force-pushed the homar/add_authorizations_metadata branch from d6e48b8 to 4bdaae9 Compare June 4, 2025 21:30

homar force-pushed the homar/add_authorizations_metadata branch from 4bdaae9 to 91ca0d7 Compare June 4, 2025 22:30

homar force-pushed the homar/add_authorizations_metadata branch from 91ca0d7 to 21bc502 Compare June 5, 2025 20:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add tables with authorization information to system.metadata #25907

Add tables with authorization information to system.metadata #25907

Uh oh!

homar commented Jun 2, 2025 •

edited

Loading

Uh oh!

lukasz-walkiewicz Jun 4, 2025

Uh oh!

lukasz-walkiewicz Jun 4, 2025

Uh oh!

lukasz-walkiewicz Jun 4, 2025

Uh oh!

lukasz-walkiewicz Jun 4, 2025

Uh oh!

homar Jun 4, 2025

Uh oh!

lukasz-walkiewicz Jun 4, 2025

Uh oh!

homar Jun 4, 2025

Uh oh!

kokosing Jun 4, 2025

Uh oh!

homar Jun 4, 2025

Uh oh!

kokosing Jun 4, 2025

Uh oh!

kokosing Jun 4, 2025

Uh oh!

homar Jun 4, 2025

Uh oh!

Uh oh!

Add tables with authorization information to system.metadata #25907

Are you sure you want to change the base?

Add tables with authorization information to system.metadata #25907

Uh oh!

Conversation

homar commented Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Additional context and related issues

Release notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

homar commented Jun 2, 2025 •

edited

Loading