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

release-20.2: sql: fix schema privileges on descriptor read #65999

Merged

Conversation

RichardJCai
Copy link
Contributor

Backport 1/1 commits from #65750.

/cc @cockroachdb/release


Release note (bug fix): Previously a schema's privilege descriptor
could become corrupted upon executing ALTER DATABASE ...
CONVERT TO SCHEMA due to privileges that are invalid on a schema
being copied over to the schema rendering the schema inusable due
to invalid privileges.

Fixes #65697

@RichardJCai RichardJCai requested review from ajwerner and a team June 2, 2021 17:28
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@RichardJCai
Copy link
Contributor Author

Note: this is quite different from #65750, I had to add a Validate method to SchemaDescriptors.

Validate for schema is called for all places where Validate for DatabaseDescriptors are called except getCachedDatabaseDesc has no equivalent for SchemaDescriptor (I believe)

Release note (bug fix): Previously a schema's privilege descriptor
could become corrupted upon executing ALTER DATABASE ...
CONVERT TO SCHEMA due to privileges that are invalid on a schema
being copied over to the schema rendering the schema inusable due
to invalid privileges.
@RichardJCai RichardJCai force-pushed the backport20.2-65750 branch from e90d6e0 to 0b1e366 Compare June 2, 2021 18:47
Copy link
Contributor

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

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

:lgtm:

Out of curiosity, did we do anything about repairing the schemas for restore?

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

@RichardJCai
Copy link
Contributor Author

RichardJCai commented Jun 2, 2021

:lgtm:

Out of curiosity, did we do anything about repairing the schemas for restore?

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

I didn't add anything for restore on this one. I thought it was different from the previous case where restoring from 20.1 would cause an error in 20.2.

This one is somewhat different in that it'll only be an issue if they restore a corrupted schema which will stay corrupted but then gets fixed on read I believe.

I'll double check and add a restore fix if necessary.

@RichardJCai RichardJCai merged commit a49aa9c into cockroachdb:release-20.2 Jun 2, 2021
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