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

Extend table redirects applicability #11202

Open
Tracked by #11222
findepi opened this issue Feb 25, 2022 · 6 comments
Open
Tracked by #11222

Extend table redirects applicability #11202

findepi opened this issue Feb 25, 2022 · 6 comments

Comments

@findepi
Copy link
Member

findepi commented Feb 25, 2022

Apply table redirections in more statements.

reusing @phd3 comment #10577 (comment)

IMO for every operation, we'd want to either (1) use redirected table handle OR (2) fail if a table is getting redirected. this would be safer than silently trying to get source table handle, especially for future usages. @findepi @losipiuk @electrum

If we've consensus on that, we can start with callsites where we know we want to definitely redirect. Then, separately, add a change that disallows redirection for "vanilla" Metadata#getTableHandle (or introduce a Metadata#getOriginalTableHandle).

table with some further amendments

operation use redirected table handle / disallow if redirected why? ACL checks
comment table use redirected table handle coherent with future describe / show create check on target
comment column use redirected table handle coherent with future describe / show create check on target
Add column use redirected table handle coherent with future select check on target
create table do not follow redirect when checking existence shouldn't create a table if it resolves to a "redirected" table check on source (only option)
drop column use redirected table handle coherent with future select check on target
drop table use redirected table handle (checking existence) drop table x failure --> describe table x success is confusing check on target
grant privilege, deny privilege disallow if redirected limited connector support, listPrivileges API needs to change check on target
rename table use redirected table handle¹ should source / target / everything in between be renamed? simpler to disallow check on target
revoke disallow if redirected limited connector support, listPrivileges API needs to change check on target
set table authorization ???² disallow if redirected
rename column use redirected table handle coherent with future select check on target
applyTableScanRedirect allowed this is unrelated, put here for reference only
extract spatial joins use redirected table handle name comes from SP, seems fine either way. may require some refactoring seems missing currently
show create table use redirected table handle coherent with future select
show grants disallow if redirected

¹) #10577 (comment)

²) #10577 (comment) cc @kokosing

@findepi
Copy link
Member Author

findepi commented Feb 25, 2022

I put the table above using #10577 (comment) as reference + further modifications based on the discussion there.

@electrum @phd3 @losipiuk @sopel39 @findinpath @kokosing please review

@kokosing
Copy link
Member

set table authorization

I think this should be coherent with grant and revoke

@findepi
Copy link
Member Author

findepi commented Feb 28, 2022

Thanks @kokosing for confirming. Updated above table.

@phd3
Copy link
Member

phd3 commented Feb 28, 2022

thanks @findepi. lgtm.

@findepi
Copy link
Member Author

findepi commented Mar 2, 2022

rename table | use redirected table handle¹

This depends on #11281 being accepted as a bug and resolved.

@findinpath
Copy link
Contributor

@findepi please add in the table above also DENY command (I am assuming that it should follow the same policies as GRANT and REVOKE)

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

No branches or pull requests

4 participants