Skip to content

Commit

Permalink
sql/catalog/descs: don't hydrate dropped tables
Browse files Browse the repository at this point in the history
The invariant that types referenced by tables only exists for non-dropped
tables. We were not checking the state of the table when choosing to hydrate.
This lead to pretty catastropic failures when the invariant was violated.

Fixes #54343.

Release note (bug fix): Fixed bug from earlier alphas where dropping a database
which contained tables using user-defined types could result in panics.
  • Loading branch information
ajwerner committed Sep 15, 2020
1 parent c1019fb commit c8319cf
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
10 changes: 9 additions & 1 deletion pkg/sql/catalog/descs/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -1083,13 +1083,17 @@ func (tc *Collection) resolveSchemaByID(
// hydrateTypesInTableDesc installs user defined type metadata in all types.T
// present in the input TableDescriptor. It always returns the same type of
// TableDescriptor that was passed in. It ensures that ImmutableTableDescriptors
// are not modified during the process of metadata installation.
// are not modified during the process of metadata installation. Dropped tables
// do not get hydrated.
//
// TODO(ajwerner): This should accept flags to indicate whether we can resolve
// offline descriptors.
func (tc *Collection) hydrateTypesInTableDesc(
ctx context.Context, txn *kv.Txn, desc catalog.TableDescriptor,
) (catalog.TableDescriptor, error) {
if desc.Dropped() {
return desc, nil
}
switch t := desc.(type) {
case *tabledesc.Mutable:
// It is safe to hydrate directly into Mutable since it is
Expand Down Expand Up @@ -1602,6 +1606,10 @@ func HydrateGivenDescriptors(ctx context.Context, descs []catalog.Descriptor) er
// Now hydrate all table descriptors.
for i := range descs {
desc := descs[i]
// Never hydrate dropped descriptors.
if desc.Dropped() {
continue
}
if tblDesc, ok := desc.(*tabledesc.Immutable); ok {
if err := typedesc.HydrateTypesInTableDescriptor(
ctx,
Expand Down
18 changes: 18 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/enums
Original file line number Diff line number Diff line change
Expand Up @@ -1202,3 +1202,21 @@ INSERT INTO table_with_not_null_enum_no_vals VALUES (1);
statement ok
ROLLBACK; DROP TABLE table_with_not_null_enum_no_vals; DROP TYPE enum_with_no_vals;

# Regression test that hydrating descriptors does not happen on dropped
# descriptors. See #54343.

subtest dropped_database_with_enum

statement ok
CREATE DATABASE to_drop;
USE to_drop;
CREATE TYPE greeting AS ENUM ('hi');
CREATE TABLE t(a greeting);
USE defaultdb;
DROP DATABASE to_drop CASCADE;

# Before the bug-fix which introduced this test, this call would load all
# descriptors, including dropped ones, and hydrate them, causing a panic as
# the referenced type no longer exists.
statement ok
SELECT * FROM crdb_internal.tables;

0 comments on commit c8319cf

Please sign in to comment.