sql: drop schema can lead to dangling function references

Previously, the DROP SCHEMA CASCADE command could leave dangling
function references. This would cause tables or functions that
referenced functions in the dropped schema to become unusable. The issue
stemmed from the legacy schema changer not preventing the deletion of
functions with cross-schema dependencies. This patch addresses the
problem by blocking such drops from executing with a dependent object
error.

The declarative schema changer implements this cascade behaviour
properly.

Fixes: #120353
Release note (bug fix): DROP SCHEMA CASCADE could lead to dangling
function references in other schemas accessing any functions.

pkg/sql/drop_cascade.go

@@ -12,6 +12,7 @@ package sql
 
 import (
 	"context"
+	"strings"
 
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
@@ -89,7 +90,9 @@ func (d *dropCascadeState) collectObjectsInSchema(
 // This resolves objects for DROP SCHEMA and DROP DATABASE ops.
 // db is used to generate a useful error message in the case
 // of DROP DATABASE; otherwise, db is nil.
-func (d *dropCascadeState) resolveCollectedObjects(ctx context.Context, p *planner) error {
+func (d *dropCascadeState) resolveCollectedObjects(
+	ctx context.Context, dropDatabase bool, p *planner,
+) error {
 	d.td = make([]toDelete, 0, len(d.objectNamesToDelete))
 	// Resolve each of the collected names.
 	for i := range d.objectNamesToDelete {
@@ -191,6 +194,37 @@ func (d *dropCascadeState) resolveCollectedObjects(ctx context.Context, p *plann
 		}
 	}
 
+	// Validate dropping any function will not break other objects.
+	if !dropDatabase {
+		for _, fn := range d.functionsToDelete {
+			// If any of the dependencies are in a different schema (non-dropped) our
+			// cascade support will lead to broken / inaccessible tables. If we are in the
+			// legacy schema changer world return an error. The declarative schema changer
+			// knows how to handle function cascades.
+			var idsInOtherSchemas []descpb.ID
+			for _, dependedOnBy := range fn.DependedOnBy {
+				dependedOnByDesc, err := p.Descriptors().ByID(p.Txn()).Get().Desc(ctx, dependedOnBy.ID)
+				if err != nil {
+					return err
+				}
+				if dependedOnByDesc.GetParentSchemaID() != fn.ParentSchemaID {
+					idsInOtherSchemas = append(idsInOtherSchemas, dependedOnBy.ID)
+				}
+			}
+
+			if len(idsInOtherSchemas) > 0 {
+				fullyQualifiedNames, err := p.getFullyQualifiedNamesFromIDs(ctx, idsInOtherSchemas)
+				if err != nil {
+					return err
+				}
+				return pgerror.Newf(
+					pgcode.DependentObjectsStillExist,
+					"cannot drop function %q because other object ([%v]) still depend on it",
+					fn.Name, strings.Join(fullyQualifiedNames, ", "))
+			}
+		}
+	}
+
 	allObjectsToDelete, implicitDeleteMap, err := p.accumulateAllObjectsToDelete(ctx, d.td)
 	if err != nil {
 		return err

pkg/sql/drop_database.go

@@ -108,7 +108,7 @@ func (p *planner) DropDatabase(ctx context.Context, n *tree.DropDatabase) (planN
 		}
 	}
 
-	if err := d.resolveCollectedObjects(ctx, p); err != nil {
+	if err := d.resolveCollectedObjects(ctx, true /*dropDatabase*/, p); err != nil {
 		return nil, err
 	}
 

pkg/sql/drop_schema.go

@@ -117,7 +117,7 @@ func (p *planner) DropSchema(ctx context.Context, n *tree.DropSchema) (planNode,
 
 	}
 
-	if err := d.resolveCollectedObjects(ctx, p); err != nil {
+	if err := d.resolveCollectedObjects(ctx, false /*dropDatabase*/, p); err != nil {
 		return nil, err
 	}
 

pkg/sql/logictest/testdata/logic_test/drop_function

@@ -289,12 +289,19 @@ CREATE SCHEMA altSchema;
 statement ok
 ALTER FUNCTION f_called_by_b_1 SET SCHEMA altSchema;
 
+skipif config local-legacy-schema-changer
 statement ok
 DROP SCHEMA altSchema CASCADE;
 
+onlyif config local-legacy-schema-changer
+statement error pgcode 2BP01 cannot drop function \"f_called_by_b_1\" because other object \(\[test.public.f_called_by_b_2, test.public.t1_with_b_2_ref\]\) still depend on it
+DROP SCHEMA altSchema CASCADE;
+
+skipif config local-legacy-schema-changer
 statement error pgcode 42883 unknown function: f_called_by_b_2()
 SELECT * FROM  f_called_by_b_2();
 
+skipif config local-legacy-schema-changer
 query T
 SELECT create_statement FROM [SHOW CREATE TABLE t1_with_b_2_ref];
 ----
@@ -304,5 +311,10 @@ CREATE TABLE public.t1_with_b_2_ref (
   CONSTRAINT t1_with_b_2_ref_pkey PRIMARY KEY (rowid ASC)
 )
 
+onlyif config local-legacy-schema-changer
+statement ok
+DROP FUNCTION f_called_by_b_2;
+DROP TABLE t1_with_b_2_ref;
+
 statement ok
 DROP FUNCTION f_b;