sql: fix drop schema bug which corrupts schemas using the name of the db#63119
Merged
craig[bot] merged 2 commits intocockroachdb:masterfrom Apr 6, 2021
Merged
Conversation
Member
rafiss
approved these changes
Apr 6, 2021
Collaborator
rafiss
left a comment
There was a problem hiding this comment.
lgtm! the path/filepath import needs to be removed from rsg_test, and it looks good to go
…ress The testdata is copied over to the stress nodes. Running these rsg tests at scale reveals problems rapidly. Release note: None
This bug has obscure impact but is rather egregious. When dropping a schema
we would add entry with the name of it's parent database rather than itself
into the database's name map. This generally means we'll have an orphaned
entry in that map corresponding to the database itself. That's only going to
be a problem if you want to create a schema of the same name as the database.
We can make a follow-up patch to have the code ignore entries (remove at
deserialization time) that correspond to the database itself and then we
can write a long-running migration. For now we're going to fix the root
cause.
This bug was exposed by cross-descriptor validation of schemas. Prior
to this fix, you'd get an ugly error like the following:
```
(XX000) internal error: schema "samename" (155): not present in parent database [152] schemas mapping
schema_desc.go:190: in ValidateCrossReferences()
DETAIL: stack trace:
```
I am worried that we're going to see support issues for this and we'll need
a follow-up change to do some automated repair or robustness to this problem.
Release note (bug fix): Fixed a bug in user-defined schemas whereby the
dropping of any schema may prevent creation of schemas with the name of
the database and may corrupt already existing schemas of that name.
c2c2089 to
70970fe
Compare
Contributor
Author
|
bors r=rafiss |
Contributor
|
Build succeeded: |
craig bot
pushed a commit
that referenced
this pull request
Aug 16, 2021
68567: colrpc: enhance warnings from the outbox r=yuzefovich a=yuzefovich This commit marks several string constants as "safe" from the redactability perspective so that the warnings logged by the outboxes are more helpful. Additionally, several minor nits around error formatting are addressed. Release note: None 68675: sql: implement IterateStatementStats() for PersistedSQLStats r=maryliag a=Azhng Depends on: #68555, #68620 Related to: #64743 Previously, IterateStatementStats() for PersistedSQLStats was left unimplemented and it defaults to the implementation of SQLStats.IterateStatementStats(). This means calls to IterateStatementStats() on PersistedSQLStats cannot read the statement statistitcs stored in system table. This commit implements the IterateStatementStats() through the new CombinedStmtStatsIterator which enables this method to read both in-memory and persited statement statistics. Release note: None 68738: sql/catalog/dbdesc: repair old descriptors corrupted due to DROPPED SCHEMA name r=ajwerner a=sajjadrizvi #63119 fixes a bug that corrupts a database descriptor when a child schema was dropped, adding an entry in schema-info structure erroneously with database name instead of schema name . Although the bug was fixed , there can be database backups with corrupted descriptors. This commit adds a post-deserialization function to repair a corrupted descriptor. Moreover, it adds a test function to ensure that descriptors with such corruption are fixed during migration. Release note: None Fixes: #63148 68903: github: redirect KV code reviews to @cockroachdb/kv-prs r=irfansharif a=irfansharif We try to use the @cockroachdb/kv alias for notifying the entire team on issues. There's no way to disable notifications for github's automatic codeowners driven review requests, and that tends to be a firehose, so lets use this sub-team alias instead. Release note: None 68978: changefeedccl: detect sink URLs with no scheme r=HonoreDB a=stevendanna Previously, if a user provided a sink URL with no scheme (such as ` kafka%3A%2F%2Fnope%0A`), a changefeed job would be started. However, this changefeed job would be writing into a bufferSink. The bufferSink is used by core changefeeds. The user may have provided such a URL because of confusion over how to URL encode their sink URL. Now, they will receive an error such as ``` pq: no scheme found for sink URL "kafka%3A%2F%2Fnope%0A" ``` Release note (enterprise change): CHANGEFEED statements now error if the provided sink URL does not contain a scheme. Such URLs are typically a mistake and will result in non-functional changefeeds. Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com> Co-authored-by: Azhng <archer.xn@gmail.com> Co-authored-by: Sajjad Rizvi <sajjad@cockroachlabs.com> Co-authored-by: irfan sharif <irfanmahmoudsharif@gmail.com> Co-authored-by: Steven Danna <danna@cockroachlabs.com>
This file contains hidden or 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
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.
This bug has obscure impact but is rather egregious. When dropping a schema
we would add entry with the name of it's parent database rather than itself
into the database's name map. This generally means we'll have an orphaned
entry in that map corresponding to the database itself. That's only going to
be a problem if you want to create a schema of the same name as the database.
We can make a follow-up patch to have the code ignore entries (remove at
deserialization time) that correspond to the database itself and then we
can write a long-running migration. For now we're going to fix the root
cause.
This bug was exposed by cross-descriptor validation of schemas. Prior
to this fix, you'd get an ugly error like the following:
I am worried that we're going to see support issues for this (starting in 20.2) and we'll need
a follow-up change to do some automated repair or robustness to this problem.
Fixes #62920
Release note (bug fix): Fixed a bug in user-defined schemas whereby the
dropping of any schema may prevent creation of schemas with the name of
the database and may corrupt already existing schemas of that name.