Merge #76381

76381: sql: stale descriptor state could be observed in crdb_internal tables r=fqazi a=fqazi

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

Co-authored-by: Faizan Qazi <[email protected]>

pkg/sql/catalog/descs/collection.go

@@ -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)
 }
 

pkg/sql/catalog/descs/descriptor.go

@@ -341,7 +341,6 @@ func (tc *Collection) withReadFromStore(
 		}
 		descs[i] = desc
 	}
-	tc.kv.releaseAllDescriptors()
 	return descs, nil
 }
 

pkg/sql/logictest/testdata/logic_test/table

@@ -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;