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,backupccl: set system table user ID columns to be NOT NULL #98958

Merged
merged 7 commits into from
Mar 21, 2023

Conversation

andyyang890
Copy link
Collaborator

@andyyang890 andyyang890 commented Mar 18, 2023

This PR sets the user ID columns in system tables to be NOT NULL
and when applicable, updates the RESTORE logic to account for
the case where a backup may have been created before the user ID
column was added.

Part of #87079

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@andyyang890 andyyang890 force-pushed the system_privileges_notnull branch 4 times, most recently from b062dcf to c59d196 Compare March 19, 2023 21:09
@andyyang890 andyyang890 changed the title sql,backupccl: set system.privileges user_id column to NOT NULL sql,backupccl: set system table user ID columns to be NOT NULL Mar 19, 2023
@andyyang890 andyyang890 force-pushed the system_privileges_notnull branch 13 times, most recently from 4b82578 to af43e9e Compare March 20, 2023 17:59
This patch adds the EmptyRole, which is used within some system tables.
It also assigns this role the fixed ID of 0.

Release note: None
This patch updates the `RESTORE` logic for the `system.privileges`
table to handle restoring a backup that was created before the new
`user_id` column was added.

Release note: None
This patch updates the `RESTORE` logic for the `system.database_role_settings`
table to handle restoring a backup that was created before the new
`role_id` column was added.

Release note: None
This patch updates the `RESTORE` logic for the `system.external_connections`
table to handle restoring a backup that was created before the new
`owner_id` column was added.

Release note: None
@andyyang890 andyyang890 force-pushed the system_privileges_notnull branch from af43e9e to 649104b Compare March 20, 2023 18:08
This patch sets the user ID columns in system tables to be NOT NULL
and when applicable, updates the migrations that added the columns.

Release note: None
@andyyang890 andyyang890 force-pushed the system_privileges_notnull branch from 649104b to d3268f9 Compare March 20, 2023 19:07
@andyyang890 andyyang890 marked this pull request as ready for review March 20, 2023 19:15
@andyyang890 andyyang890 requested a review from a team as a code owner March 20, 2023 19:15
@andyyang890 andyyang890 requested a review from a team as a code owner March 20, 2023 19:15
@andyyang890 andyyang890 requested a review from a team March 20, 2023 19:15
@andyyang890 andyyang890 requested review from a team as code owners March 20, 2023 19:15
@andyyang890 andyyang890 requested review from rhu713, rafiss, stevendanna, knz and a team and removed request for a team March 20, 2023 19:15
Copy link
Collaborator

@rafiss rafiss left a comment

Choose a reason for hiding this comment

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

thanks for breaking this down into small commits. it made it very easy to follow. lgtm!

could use a sign-off from the DR team as well.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @knz, @rhu713, and @stevendanna)

@knz
Copy link
Contributor

knz commented Mar 20, 2023

very nice, thank you 💯

Copy link
Collaborator

@stevendanna stevendanna left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@andyyang890
Copy link
Collaborator Author

TFTRs!

bors r=rafiss,stevendanna

@craig
Copy link
Contributor

craig bot commented Mar 20, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Mar 21, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Mar 21, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Mar 21, 2023

Build succeeded:

@craig craig bot merged commit 1cdcf18 into cockroachdb:master Mar 21, 2023
@andyyang890 andyyang890 deleted the system_privileges_notnull branch March 21, 2023 07:37
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.

5 participants