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 dropping non-owned descendants #18873

Merged
merged 1 commit into from
Apr 20, 2023

Conversation

jkosh44
Copy link
Contributor

@jkosh44 jkosh44 commented Apr 20, 2023

This commit reverts the changes made in
acc39ea. Specifically it allows dropping non-owned objects with a CASCADE statement. This aligns with the behavior in PostgreSQL. Roles are not allowed to directly drop objects if they don't own the object, but they can drop those objects indirectly through CASCADE. The other of
acc39ea, who's also the author of this commit, misunderstood the intended behavior.

Works towards resolving #18551
Part of #11579

Motivation

This PR fixes a recognized bug.

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered.
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • This PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way) and therefore is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • This PR includes the following user-facing behavior changes:
    • This release fixes the behavior of dropping non-owned objects through CASCADE.

This commit reverts the changes made in
acc39ea. Specifically it allows
dropping non-owned objects with a CASCADE statement. This aligns with
the behavior in PostgreSQL. Roles are not allowed to directly drop
objects if they don't own the object, but they can drop those objects
indirectly through CASCADE. The other of
acc39ea, who's also the author of
this commit, misunderstood the intended behavior.

Works towards resolving MaterializeInc#18551
Part of MaterializeInc#11579
@jkosh44 jkosh44 requested a review from def- April 20, 2023 17:33
@jkosh44
Copy link
Contributor Author

jkosh44 commented Apr 20, 2023

Another option would be to directly revert acc39ea, but this ends up being much easier.

@jkosh44 jkosh44 marked this pull request as ready for review April 20, 2023 18:06
@jkosh44 jkosh44 requested a review from a team as a code owner April 20, 2023 18:06
@jkosh44 jkosh44 merged commit 41a4dea into MaterializeInc:main Apr 20, 2023
@jkosh44 jkosh44 deleted the dependent-drops branch April 20, 2023 22:33
joacoc pushed a commit to joacoc/materialize that referenced this pull request Apr 21, 2023
This commit reverts the changes made in
acc39ea. Specifically it allows
dropping non-owned objects with a CASCADE statement. This aligns with
the behavior in PostgreSQL. Roles are not allowed to directly drop
objects if they don't own the object, but they can drop those objects
indirectly through CASCADE. The author of
acc39ea, who's also the author of this
commit, misunderstood the intended behavior.

Works towards resolving MaterializeInc#18551
Part of MaterializeInc#11579
@jkosh44 jkosh44 mentioned this pull request Apr 24, 2023
1 task
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.

None yet

2 participants