Skip to content

Commit

Permalink
sql: stale descriptor state could be observed in crdb_internal tables
Browse files Browse the repository at this point in the history
Fixes: #75825

Previously, when an uncommitted descriptor was added
to a descriptor collection, we would not reset the cache of
all immutable descriptors (used by some crdb_internal functions).
This was inadequate because this could result in
incorrect results from crdb_internal tables which returned
information related to constraints, columns. To address this,
this patch will reset the descriptor cache for all
fetched descriptors when an uncommitted descriptor is
added into the collection.

Release note: None
  • Loading branch information
fqazi committed Feb 10, 2022
1 parent d10188f commit bb0d19e
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 1 deletion.
3 changes: 3 additions & 0 deletions pkg/sql/catalog/descs/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@ var _ = (*Collection).HasUncommittedTypes
// immutably will return a copy of the descriptor in the current state. A deep
// copy is performed in this call.
func (tc *Collection) AddUncommittedDescriptor(desc catalog.MutableDescriptor) error {
// Invalidate all the cached descriptors since a stale copy of this may be
// included.
tc.kv.releaseAllDescriptors()
return tc.uncommitted.checkIn(desc)
}

Expand Down
1 change: 0 additions & 1 deletion pkg/sql/catalog/descs/descriptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,6 @@ func (tc *Collection) withReadFromStore(
}
descs[i] = desc
}
tc.kv.releaseAllDescriptors()
return descs, nil
}

Expand Down
39 changes: 39 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/table
Original file line number Diff line number Diff line change
Expand Up @@ -855,3 +855,42 @@ query I
SELECT estimated_row_count FROM [SHOW TABLES from rowtest] where table_name = 't1'
----
1000

# Tests that the collections cache is properly invalidated when uncommitted
# descriptors are added as observed in #75825.
subtest collection-improper-release

statement ok
BEGIN;
CREATE TABLE tblmodified (price INT8, quantity INT8);
ALTER TABLE tblmodified ADD CONSTRAINT quan_check CHECK (quantity > 0);
ALTER TABLE tblmodified ADD CONSTRAINT pr_check CHECK (price > 0);

# Check out the descriptor for tblmodified under the all descriptors cache,
query TTB
SELECT conname,
pg_get_constraintdef(c.oid) AS constraintdef,
c.convalidated AS valid
FROM pg_constraint AS c JOIN pg_class AS t ON c.conrelid = t.oid
WHERE c.contype = 'c' AND t.relname = 'tblmodified' ORDER BY conname ASC;
----
pr_check CHECK ((price > 0)) true
quan_check CHECK ((quantity > 0)) true

# Descriptor is modified, so the cache should be invalidated.
statement ok
ALTER TABLE tblmodified DROP CONSTRAINT quan_check;

# If the cache is properly invalidated then we should only see a single
# constraint.
query TTB
SELECT conname,
pg_get_constraintdef(c.oid) AS constraintdef,
c.convalidated AS valid
FROM pg_constraint AS c JOIN pg_class AS t ON c.conrelid = t.oid
WHERE c.contype = 'c' AND t.relname = 'tblmodified' ORDER BY conname ASC;
----
pr_check CHECK ((price > 0)) true

statement ok
COMMIT;

0 comments on commit bb0d19e

Please sign in to comment.