backupccl: skip UDFs not found when rewriting IDs in Schema #96911
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, if a BACKUP is created by doing BACKUP TABLE, and the parent schema of the tables has UDFs in it, RESTOREing the tables from the back crashes. This is because we store the signatures of UDFs in schema descriptor. Restoring a table also restores the parent schema if there is no schema with the same name exists in the target database. So when trying to rewrite the UDF ids in the schema descriptor, it crashes because function descriptors are not backed up at all. This pr adds logic to skip those rewrites if function descriptor was not backed up. Fixes: cockroachdb#96910 Release note (enterprise): this path fixes a bug where server would crash if trying to restore a table from a backup generated with `BACKUP TABLE` from a schema with user defined functions, and the restore target database doesn't have a schema with the same name.
chengxiong-ruan
requested review from
a team and
benbardin
and removed request for
a team
|
This change is |
fqazi
approved these changes
Feb 10, 2023
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @chengxiong-ruan)
|
TFTR! |
|
Build succeeded: |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from c29c242 to blathers/backport-release-22.2-96911: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 22.2.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
chengxiong-ruan
added a commit
to chengxiong-ruan/cockroach
that referenced
this pull request
Feb 10, 2023
Forgot to assign the new function signatures to schema in cockroachdb#96911. This pr fixes that and also added more tests to make sure functions in schema descriptor looks good. Epic: None Release note: None
chengxiong-ruan
added a commit
to chengxiong-ruan/cockroach
that referenced
this pull request
Feb 10, 2023
Forgot to assign the new function signatures to schema in cockroachdb#96911. This patch fixes that and also added more tests to make sure functions in schema descriptor looks good. Epic: None --- Release note: None ---
craig bot
pushed a commit
that referenced
this pull request
Feb 10, 2023
96648: schemachanger: implement `ALTER TABLE .. VALIDATE CONSTRAINT ..` r=Xiang-Gu a=Xiang-Gu The main idea is to drop the unvalidated element and add a vanilla counterpart in the builder state. Commit 2 involves some changes to allow both legacy and declarative schema changer handle this stmt correctly: `ALTER TABLE .. ADD/DROP CONSTRAINT .. [NOT VALID]; VALIDATE CONSTRAINT ..;` so this PR warrants a release note. Epic: None Release note (bug fix): Allow `ALTER TABLE .. ADD/DROP CONSTRAINT .. [NOT VALID]; VALIDATE CONSTRAINT ..;` to behave consistently with Postgres. Previously, the VALIDATE CONSTRAINT part will fail and cause the whole statement to fail. 96983: backupccl: reassign functions for rewritten schemas r=chengxiong-ruan a=chengxiong-ruan Forgot to assign the new function signatures to schema in #96911. This pr fixes that and also added more tests to make sure functions in schema descriptor looks good. Epic: None Release note: None Co-authored-by: Xiang Gu <[email protected]> Co-authored-by: Chengxiong Ruan <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, if a BACKUP is created by doing BACKUP TABLE, and the parent schema of the tables has UDFs in it, RESTOREing the tables from the back crashes. This is because we store the signatures of UDFs in schema descriptor. Restoring a table also restores the parent schema if there is no schema with the same name exists in the target database. So when trying to rewrite the UDF ids in the schema descriptor, it crashes because function descriptors are not backed up at all.
This pr adds logic to skip those rewrites if function descriptor was not backed up.
Fixes: #96910
Release note (enterprise): this path fixes a bug where server would crash if trying to restore a table from a backup generated with
BACKUP TABLEfrom a schema with user defined functions, and the restore target database doesn't have a schema with the same name.