Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ALTER FUNCTION ... SET AUTHORIZATION #25021

Open
homar opened this issue Feb 14, 2025 · 6 comments
Open

Add ALTER FUNCTION ... SET AUTHORIZATION #25021

homar opened this issue Feb 14, 2025 · 6 comments
Assignees

Comments

@homar
Copy link
Member

homar commented Feb 14, 2025

Trino already supports SET AUTHORIZATION fors schemas, tables and views.
It is missing for functions so I propose to add it with following syntax:

ALTER FUNCTION function_name SET AUTHORIZATION ( user | USER user | ROLE role )
@kokosing
Copy link
Member

To me this syntax is reasonable. It follows the similar syntax that we have for tables, views and schema. I also understand that the similar access control checks will need to be also implemented to those we have today.

@martint are you ok with this syntax?

@findinpath
Copy link
Contributor

cc @djsstarburst

@mosabua
Copy link
Member

mosabua commented Feb 14, 2025

Sounds reasonable to get started .. however I think if we engage on that work, we can't just stop there. We should flesh out all the syntax we want to support in a roadmap item (maybe this one) and then start implementing whatever we need first.

@homar
Copy link
Member Author

homar commented Feb 17, 2025

Sounds reasonable to get started .. however I think if we engage on that work, we can't just stop there. We should flesh out all the syntax we want to support in a roadmap item (maybe this one) and then start implementing whatever we need first.

@mosabua I created more general ticket here: #25042

@djsstarburst
Copy link
Member

I have this PR that adds generic SET AUTHORIZATION, in a similar way that I added generic GRANT/DENY/REVOKE. @martint looked at it early on, and @dain went a couple of cycles reviewing it right before Christmas. I expected him to merge the PR, but I think he had other things on his mind ☹

I'll ping him again.

@dain
Copy link
Member

dain commented Feb 18, 2025

I was just thinking "I swear striker already added code for this". I'll take a look at the PR again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

7 participants