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

sql: support RULE privilege for compatibility #73960

Merged
merged 1 commit into from Dec 17, 2021

Conversation

rafiss
Copy link
Collaborator

@rafiss rafiss commented Dec 17, 2021

fixes #72832

The RULE privilege is legacy in Postgres, but some tools expect
the has_table_privilege function to work.

Release note (sql change): The RULE privilege was added for compatbility
with Postgres. It is impossible to grant it, but it is supported as a
parameter of the has_table_privilege function.

@rafiss rafiss requested review from otan and a team December 17, 2021 05:19
@cockroach-teamcity
Copy link
Member

This change is Reviewable

----
true true true true true true true true true
true true true true true true true true true true
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm still thinking about this -- some of these return true for RULE because the user has ALL privileges. i'm wondering if we should special-case RULE in the has_table_privilege implementation so it just always returns false.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @otan ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i changed it to always return false

The RULE privilege is legacy in Postgres, but some tools expect
the has_table_privilege function to work.

Release note (sql change): The RULE privilege was added for compatbility
with Postgres. It is impossible to grant it, but it is supported as a
parameter of the has_table_privilege function.
@otan
Copy link
Contributor

otan commented Dec 17, 2021

I would do what postgres doesin the ALL case. I understand the argument both ways but I guess when I come back to is that if we're following pg compat then we should match pg.

@rafiss
Copy link
Collaborator Author

rafiss commented Dec 17, 2021

tftr! merging after unrelated flake

bors r=otan

@craig
Copy link
Contributor

craig bot commented Dec 17, 2021

Build succeeded:

@craig craig bot merged commit 7559f51 into cockroachdb:master Dec 17, 2021
@rafiss rafiss deleted the add-rule-priv branch December 17, 2021 20:55
@rafiss
Copy link
Collaborator Author

rafiss commented Dec 17, 2021

can this be backported to 21.2? it would unblock a customer who's trying to use a tool that relies on this, but not sure how risky it is

@otan
Copy link
Contributor

otan commented Dec 18, 2021

don't see why not

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

Successfully merging this pull request may close these issues.

sql: support has_table_privilege(oid, 'RULE')
3 participants