From 998f4db3b74b709f941ae67216ec19ff79b463ea Mon Sep 17 00:00:00 2001 From: Andrew Werner Date: Mon, 5 Apr 2021 17:39:24 -0400 Subject: [PATCH] sql: fix drop schema bug which corrupts schemas using the name of the db 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. --- pkg/sql/drop_schema.go | 4 +-- pkg/sql/logictest/testdata/logic_test/schema | 33 ++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/pkg/sql/drop_schema.go b/pkg/sql/drop_schema.go index 84a09dd52c27..78d8f939147d 100644 --- a/pkg/sql/drop_schema.go +++ b/pkg/sql/drop_schema.go @@ -174,8 +174,8 @@ func (p *planner) dropSchemaImpl( if parentDB.Schemas == nil { parentDB.Schemas = make(map[string]descpb.DatabaseDescriptor_SchemaInfo) } - parentDB.Schemas[parentDB.Name] = descpb.DatabaseDescriptor_SchemaInfo{ - ID: parentDB.ID, + parentDB.Schemas[sc.GetName()] = descpb.DatabaseDescriptor_SchemaInfo{ + ID: sc.GetID(), Dropped: true, } // Mark the descriptor as dropped. diff --git a/pkg/sql/logictest/testdata/logic_test/schema b/pkg/sql/logictest/testdata/logic_test/schema index f7860d9f1af9..ab0ff47af43f 100644 --- a/pkg/sql/logictest/testdata/logic_test/schema +++ b/pkg/sql/logictest/testdata/logic_test/schema @@ -611,3 +611,36 @@ query TT SELECT schema_name, table_name FROM [SHOW TABLES FROM for_show.sc1] ---- sc1 t1 + +# Regression test for #62920. The bug that motivated this test would populate +# the schema entry in the database with the database's name rather than the +# schemas. +subtest schema_and_database_with_same_name + +statement ok +CREATE DATABASE samename + +statement ok +USE samename + +statement ok +CREATE SCHEMA foo; +CREATE SCHEMA bar + +statement ok +DROP SCHEMA foo + +statement ok +CREATE SCHEMA samename + +statement ok +DROP SCHEMA bar + +statement ok +CREATE TABLE samename.samename.t (i INT PRIMARY KEY) + +statement ok +SHOW TABLES + +statement ok +DROP DATABASE samename CASCADE;