sql: ban renaming table to a new database unless both schemas are public#55722
Conversation
|
This change is |
ajwerner
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @ajwerner and @lucy-zhang)
pkg/sql/rename_table.go, line 180 at r1 (raw file):
// Note that we don't have to modify the parent schema ID because the source // and target schemas must both be the public schema.
Where is this enforced? The above check doesn't say anything about the parentSchemaID if the catalogs are the same.
Previously we would seemingly allow renaming tables to a new database with arbitrary source and target schemas without ever actually reassigning the schema ID. This could lead to a corrupted descriptor with an invalid schema ID for the database. This PR fixes the implementation to return an error when renaming a table to a different database unless both the source and target schemas are `public`. Release note (bug fix): Previously, changing the parent database and schema of a table using `RENAME` was seemingly permitted but would lead to corruption of the table metadata. Now an error is returned when attempting to rename a table to a different database except in the case where both the source and target schemas are the `public` schema in each database, which continues to be supported.
8bcada7 to
a978b55
Compare
thoszhang
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/rename_table.go, line 180 at r1 (raw file):
Previously, ajwerner wrote…
// Note that we don't have to modify the parent schema ID because the source // and target schemas must both be the public schema.Where is this enforced? The above check doesn't say anything about the parentSchemaID if the catalogs are the same.
I forgot about the other case while I was writing this comment. But it remains true that we never modify the schema ID because we don't allow changing the table's parent schema within the same database, either (which is checked immediately above the check added in this PR).
ajwerner
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/rename_table.go, line 180 at r1 (raw file):
Previously, lucy-zhang (Lucy Zhang) wrote…
I forgot about the other case while I was writing this comment. But it remains true that we never modify the schema ID because we don't allow changing the table's parent schema within the same database, either (which is checked immediately above the check added in this PR).
I see it now, needed to expand up. Thanks!
|
TFTR bors r+ |
|
Build failed (retrying...): |
|
Build succeeded: |
Previously we would seemingly allow renaming tables to a new database
with arbitrary source and target schemas without ever actually
reassigning the schema ID. This could lead to a corrupted descriptor
with an invalid schema ID for the database. This PR fixes the
implementation to return an error when renaming a table to a different
database unless both the source and target schemas are
public.Fixes #55710.
Release note (bug fix): Previously, changing the parent database and
schema of a table using
RENAMEwas seemingly permitted but would leadto corruption of the table metadata. Now an error is returned when
attempting to rename a table to a different database except in the case
where both the source and target schemas are the
publicschema in eachdatabase, which continues to be supported.