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: allow parsing of DROP OWNED BY #55816

Merged
merged 1 commit into from
Oct 21, 2020

Conversation

angelapwen
Copy link

@angelapwen angelapwen commented Oct 21, 2020

Part 1 of addressing #55381.

Add parsing and telemetry support for DROP OWNED BY command.
Includes small refactors of the related REASSIGN OWNED BY
command such as appending "_by" to some instances of
"reassign_owned" for readability.

Plan to follow up with implementation of DROP OWNED BY for
feature parity with PostgresQL, which will resolve the issue
linked.

Note that this command is also making use of role_spec_list
which as noted in #54696 does not properly parse
CURRENT_USER or SESSION_USER and uses strings for
roles/usernames rather than expressions. This issue is out of
scope for this PR and will be resolved separately.

Release note: None

@angelapwen angelapwen requested review from otan and a team October 21, 2020 15:11
@angelapwen angelapwen self-assigned this Oct 21, 2020
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@angelapwen angelapwen force-pushed the parse-drop-owned-by branch 2 times, most recently from 410c1d5 to 90496c6 Compare October 21, 2020 15:17
pkg/sql/opaque.go Outdated Show resolved Hide resolved
@@ -8,7 +8,6 @@ exec
CREATE ROLE testuser;
CREATE ROLE testuser2;
CREATE TABLE t();
GRANT testuser, testuser2 TO root;
Copy link
Contributor

Choose a reason for hiding this comment

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

why did we delete this line?

Copy link
Author

Choose a reason for hiding this comment

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

I removed it because root should not need to be a member of roles in order to reassign items from one to another. It was there previously because the admin privileges were not correct but @solongordon fixed it in #55273. I removed the corresponding lines in the REASSIGN OWNED BY logic test previously but just noticed that the telemetry test still has this extraneous line.

pkg/sql/sem/tree/stmt.go Outdated Show resolved Hide resolved
@angelapwen angelapwen force-pushed the parse-drop-owned-by branch 4 times, most recently from a29b291 to e3f80f1 Compare October 21, 2020 19:27
Add parsing and telemetry support for DROP OWNED BY command.
Includes small refactors of the related REASSIGN OWNED BY
command such as appending "_by" to some instances of
"reassign_owned" for readability.

Plan to follow up with implementation of DROP OWNED BY for
feature parity with PostgresQL, which will resolve the issue
linked.

Release note: None
@angelapwen
Copy link
Author

bors r=otan

@craig
Copy link
Contributor

craig bot commented Oct 21, 2020

This PR was included in a batch that successfully built, but then failed to merge into master (it was a non-fast-forward update). It will be automatically retried.

@craig
Copy link
Contributor

craig bot commented Oct 21, 2020

Build succeeded:

@craig craig bot merged commit 37188b0 into cockroachdb:master Oct 21, 2020
@angelapwen angelapwen deleted the parse-drop-owned-by branch February 5, 2021 00:16
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.

3 participants