sql,schemachanger: add remaining handling for routine references in views

This commit adds the remaining logic needed to keep track of references
to routines from a view query in the schema changer. The following commit
will resolve these references upon planning the `CREATE VIEW` statement
and pass them to the schema changer.

Informs #146123

Release note: None

Author: DrewKimball

pkg/sql/alter_function.go

@@ -210,32 +210,22 @@ func (n *alterFunctionRenameNode) startExec(params runParams) error {
 		}
 	}
 
-	// Disallow renaming if this rename operation will break other UDF's invoking
-	// this one.
-	var dependentFuncs []string
-	for _, dep := range fnDesc.GetDependedOnBy() {
-		desc, err := params.p.Descriptors().ByIDWithoutLeased(params.p.Txn()).Get().Desc(params.ctx, dep.ID)
-		if err != nil {
-			return err
-		}
-		_, ok := desc.(catalog.FunctionDescriptor)
-		if !ok {
-			continue
-		}
-		fullyResolvedName, err := params.p.GetQualifiedFunctionNameByID(params.ctx, int64(dep.ID))
-		if err != nil {
-			return err
-		}
-		dependentFuncs = append(dependentFuncs, fullyResolvedName.FQString())
+	// Disallow renaming if this rename operation will break other UDF's or views
+	// invoking this one.
+	dependentObjects, err := getFuncRefsDisallowingAlter(params, fnDesc)
+	if err != nil {
+		return err
 	}
-	if len(dependentFuncs) > 0 {
+	if len(dependentObjects) > 0 {
+		// TODO(#146475): once functions are rewritten to use ID references, we can
+		// allow renames for views.
 		return errors.UnimplementedErrorf(
 			errors.IssueLink{
 				IssueURL: build.MakeIssueURL(83233),
 				Detail:   "renames are disallowed because references are by name",
 			},
-			"cannot rename function %q because other functions ([%v]) still depend on it",
-			fnDesc.Name, strings.Join(dependentFuncs, ", "))
+			"cannot rename function %q because other functions or views ([%v]) still depend on it",
+			fnDesc.Name, strings.Join(dependentObjects, ", "))
 	}
 
 	scDesc.RemoveFunction(fnDesc.GetName(), fnDesc.GetID())
@@ -383,33 +373,21 @@ func (n *alterFunctionSetSchemaNode) startExec(params runParams) error {
 	if err != nil {
 		return err
 	}
-	// Disallow renaming if this rename operation will break other UDF's invoking
-	// this one.
-	var dependentFuncs []string
-	for _, dep := range fnDesc.GetDependedOnBy() {
-		desc, err := params.p.Descriptors().ByIDWithoutLeased(params.p.Txn()).Get().Desc(params.ctx, dep.ID)
-		if err != nil {
-			return err
-		}
-		_, ok := desc.(catalog.FunctionDescriptor)
-		if !ok {
-			continue
-		}
-		fullyResolvedName, err := params.p.GetQualifiedFunctionNameByID(params.ctx, int64(dep.ID))
-		if err != nil {
-			return err
-		}
-		dependentFuncs = append(dependentFuncs, fullyResolvedName.FQString())
+	// Disallow renaming if this rename operation will break other UDF's or views
+	// invoking this one.
+	dependentObjects, err := getFuncRefsDisallowingAlter(params, fnDesc)
+	if err != nil {
+		return err
 	}
-	if len(dependentFuncs) > 0 {
+	if len(dependentObjects) > 0 {
 		return errors.UnimplementedErrorf(
 			errors.IssueLink{
 				IssueURL: build.MakeIssueURL(83233),
 				Detail: "set schema is disallowed because there are references from " +
 					"other objects by name",
 			},
-			"cannot set schema for function %q because other functions ([%v]) still depend on it",
-			fnDesc.Name, strings.Join(dependentFuncs, ", "))
+			"cannot set schema for function %q because other functions or views ([%v]) still depend on it",
+			fnDesc.Name, strings.Join(dependentObjects, ", "))
 	}
 
 	switch sc.SchemaKind() {
@@ -548,3 +526,35 @@ func toSchemaOverloadSignature(fnDesc *funcdesc.Mutable) descpb.SchemaDescriptor
 	}
 	return ret
 }
+
+func getFuncRefsDisallowingAlter(
+	params runParams, fnDesc *funcdesc.Mutable,
+) (dependentObjects []string, err error) {
+	for _, dep := range fnDesc.GetDependedOnBy() {
+		desc, err := params.p.Descriptors().ByIDWithoutLeased(params.p.Txn()).Get().Desc(params.ctx, dep.ID)
+		if err != nil {
+			return nil, err
+		}
+		switch t := desc.(type) {
+		case catalog.TableDescriptor:
+			if !t.IsView() {
+				continue
+			}
+			fullyResolvedName, err := params.p.GetQualifiedTableNameByID(
+				params.ctx, int64(dep.ID), tree.ResolveAnyTableKind)
+			if err != nil {
+				return nil, err
+			}
+			dependentObjects = append(dependentObjects, fullyResolvedName.FQString())
+		case catalog.FunctionDescriptor:
+			fullyResolvedName, err := params.p.GetQualifiedFunctionNameByID(params.ctx, int64(dep.ID))
+			if err != nil {
+				return nil, err
+			}
+			dependentObjects = append(dependentObjects, fullyResolvedName.FQString())
+		default:
+			continue
+		}
+	}
+	return dependentObjects, nil
+}

pkg/sql/catalog/descpb/structured.proto

@@ -1838,6 +1838,9 @@ message FunctionDescriptor {
       (gogoproto.casttype) = "TriggerID"];
     repeated uint32 policy_ids = 6 [(gogoproto.customname) = "PolicyIDs",
       (gogoproto.casttype) = "PolicyID"];
+    // ViewQuery indicates that this function is referenced in the query of a
+    // view.
+    optional bool view_query = 7 [(gogoproto.nullable) = false];
   }
 
   optional string name = 1 [(gogoproto.nullable) = false];

pkg/sql/catalog/funcdesc/func_desc.go

@@ -890,6 +890,34 @@ func (desc *Mutable) AddIndexReference(id descpb.ID, indexID descpb.IndexID) err
 	return nil
 }
 
+// AddViewReference adds back reference to a view to the function.
+func (desc *Mutable) AddViewReference(id descpb.ID) error {
+	for _, dep := range desc.DependsOn {
+		if dep == id {
+			return pgerror.Newf(pgcode.InvalidFunctionDefinition,
+				"cannot add dependency from descriptor %d to function %s (%d) because there will be a dependency cycle", id, desc.GetName(), desc.GetID(),
+			)
+		}
+	}
+	for i := range desc.DependedOnBy {
+		if desc.DependedOnBy[i].ID == id {
+			desc.DependedOnBy[i].ViewQuery = true
+			return nil
+		}
+	}
+	desc.DependedOnBy = append(
+		desc.DependedOnBy,
+		descpb.FunctionDescriptor_Reference{
+			ID:        id,
+			ViewQuery: true,
+		},
+	)
+	sort.Slice(desc.DependedOnBy, func(i, j int) bool {
+		return desc.DependedOnBy[i].ID < desc.DependedOnBy[j].ID
+	})
+	return nil
+}
+
 // RemoveIndexReference removes back reference to an index from the function.
 func (desc *Mutable) RemoveIndexReference(id descpb.ID, indexID descpb.IndexID) {
 	for i := range desc.DependedOnBy {
@@ -906,6 +934,17 @@ func (desc *Mutable) RemoveIndexReference(id descpb.ID, indexID descpb.IndexID)
 	}
 }
 
+// RemoveViewReference removes back reference to a view from the function.
+func (desc *Mutable) RemoveViewReference(id descpb.ID) {
+	for i := range desc.DependedOnBy {
+		if desc.DependedOnBy[i].ID == id {
+			desc.DependedOnBy[i].ViewQuery = false
+			desc.maybeRemoveTableReference(id)
+			return
+		}
+	}
+}
+
 // maybeRemoveTableReference removes a table's references from the function if
 // the column, index and constraint references are all empty. This function is
 // only used internally when removing an individual column, index or constraint
@@ -914,7 +953,8 @@ func (desc *Mutable) maybeRemoveTableReference(id descpb.ID) {
 	var ret []descpb.FunctionDescriptor_Reference
 	for _, ref := range desc.DependedOnBy {
 		if ref.ID == id && len(ref.ColumnIDs) == 0 && len(ref.IndexIDs) == 0 &&
-			len(ref.ConstraintIDs) == 0 && len(ref.TriggerIDs) == 0 && len(ref.PolicyIDs) == 0 {
+			len(ref.ConstraintIDs) == 0 && len(ref.TriggerIDs) == 0 &&
+			len(ref.PolicyIDs) == 0 && !ref.ViewQuery {
 			continue
 		}
 		ret = append(ret, ref)

pkg/sql/create_view.go

@@ -299,12 +299,12 @@ func (n *createViewNode) startExec(params runParams) error {
 				}
 				desc.DependsOnTypes = append(desc.DependsOnTypes, orderedTypeDeps.Ordered()...)
 
-				// Collect all functions this view depends on.
-				orderedFuncDeps := catalog.DescriptorIDSet{}
+				// Collect all routines this view depends on.
+				orderedRoutineDeps := catalog.DescriptorIDSet{}
 				for backrefID := range n.funcDeps {
-					orderedFuncDeps.Add(backrefID)
+					orderedRoutineDeps.Add(backrefID)
 				}
-				desc.DependsOnFunctions = append(desc.DependsOnFunctions, orderedFuncDeps.Ordered()...)
+				desc.DependsOnFunctions = append(desc.DependsOnFunctions, orderedRoutineDeps.Ordered()...)
 
 				newDesc = &desc
 
@@ -357,6 +357,13 @@ func (n *createViewNode) startExec(params runParams) error {
 				}
 			}
 
+			// Add back references for the routine dependencies.
+			for id := range n.funcDeps {
+				if err := params.p.addRoutineViewBackReference(params.ctx, id, newDesc.ID); err != nil {
+					return err
+				}
+			}
+
 			if err := validateDescriptor(params.ctx, params.p, newDesc); err != nil {
 				return err
 			}
@@ -796,6 +803,18 @@ func (p *planner) replaceViewDesc(
 		return nil, err
 	}
 
+	// For each old function dependency (i.e. before replacing the view),
+	// see if we still depend on it. If not, then remove the back reference.
+	var outdatedRoutineRefs []descpb.ID
+	for _, id := range toReplace.DependsOnFunctions {
+		if _, ok := n.funcDeps[id]; !ok {
+			outdatedRoutineRefs = append(outdatedRoutineRefs, id)
+		}
+	}
+	if err := p.removeRoutineViewBackReferences(ctx, outdatedRoutineRefs, toReplace.ID); err != nil {
+		return nil, err
+	}
+
 	// Since the view query has been replaced, the dependencies that this
 	// table descriptor had are gone.
 	toReplace.DependsOn = make([]descpb.ID, 0, len(n.planDeps))
@@ -806,6 +825,10 @@ func (p *planner) replaceViewDesc(
 	for backrefID := range n.typeDeps {
 		toReplace.DependsOnTypes = append(toReplace.DependsOnTypes, backrefID)
 	}
+	toReplace.DependsOnFunctions = make([]descpb.ID, 0, len(n.funcDeps))
+	for backrefID := range n.funcDeps {
+		toReplace.DependsOnFunctions = append(toReplace.DependsOnFunctions, backrefID)
+	}
 
 	// Since we are replacing an existing view here, we need to write the new
 	// descriptor into place.
@@ -907,3 +930,36 @@ func crossDBReferenceDeprecationHint() string {
 	return fmt.Sprintf("Note that cross-database references will be removed in future releases. See: %s",
 		docs.ReleaseNotesURL(`#deprecations`))
 }
+
+func (p *planner) addRoutineViewBackReference(
+	ctx context.Context, routineID descpb.ID, ref descpb.ID,
+) error {
+	mutDesc, err := p.Descriptors().MutableByID(p.txn).Function(ctx, routineID)
+	if err != nil {
+		return err
+	}
+
+	if err := mutDesc.AddViewReference(ref); err != nil {
+		return err
+	}
+	if err := p.writeFuncSchemaChange(ctx, mutDesc); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (p *planner) removeRoutineViewBackReferences(
+	ctx context.Context, routineIDs []descpb.ID, ref descpb.ID,
+) error {
+	for _, routineID := range routineIDs {
+		mutDesc, err := p.Descriptors().MutableByID(p.txn).Function(ctx, routineID)
+		if err != nil {
+			return err
+		}
+		mutDesc.RemoveViewReference(ref)
+		if err := p.writeFuncSchemaChange(ctx, mutDesc); err != nil {
+			return err
+		}
+	}
+	return nil
+}

pkg/sql/drop_view.go

@@ -323,6 +323,12 @@ func (p *planner) dropViewImpl(
 		return cascadeDroppedViews, err
 	}
 
+	// Remove back-references from the routines this view depends on.
+	routinesDependedOn := append([]descpb.ID(nil), viewDesc.DependsOnFunctions...)
+	if err := p.removeRoutineViewBackReferences(ctx, routinesDependedOn, viewDesc.ID); err != nil {
+		return cascadeDroppedViews, err
+	}
+
 	if behavior == tree.DropCascade {
 		dependedOnBy := append([]descpb.TableDescriptor_Reference(nil), viewDesc.DependedOnBy...)
 		for _, ref := range dependedOnBy {

pkg/sql/schemachanger/scdecomp/decomp.go

@@ -302,6 +302,7 @@ func (w *walkCtx) walkRelation(tbl catalog.TableDescriptor) {
 			ViewID:          tbl.GetID(),
 			UsesTypeIDs:     catalog.MakeDescriptorIDSet(tbl.GetDependsOnTypes()...).Ordered(),
 			UsesRelationIDs: catalog.MakeDescriptorIDSet(tbl.GetDependsOn()...).Ordered(),
+			UsesRoutineIDs:  catalog.MakeDescriptorIDSet(tbl.GetDependsOnFunctions()...).Ordered(),
 			IsTemporary:     tbl.IsTemporary(),
 			IsMaterialized:  tbl.MaterializedView(),
 			ForwardReferences: func(tbl catalog.TableDescriptor) []*scpb.View_Reference {

pkg/sql/schemachanger/scdecomp/testdata/other

@@ -495,6 +495,7 @@ ElementState:
     isTemporary: false
     usesRelationIds:
     - 109
+    usesRoutineIds: []
     usesTypeIds: []
     viewId: 112
   Status: PUBLIC
@@ -810,6 +811,7 @@ ElementState:
     isTemporary: false
     usesRelationIds:
     - 112
+    usesRoutineIds: []
     usesTypeIds:
     - 110
     - 111

pkg/sql/schemachanger/scpb/elements.proto

@@ -475,6 +475,7 @@ message View {
   uint32 view_id = 1 [(gogoproto.customname) = "ViewID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"];
   repeated uint32 uses_type_ids = 2 [(gogoproto.customname) = "UsesTypeIDs", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"];
   repeated uint32 uses_relation_ids = 3 [(gogoproto.customname) = "UsesRelationIDs", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"];
+  repeated uint32 uses_routine_ids = 12 [(gogoproto.customname) = "UsesRoutineIDs", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"];
 
   // a reference from this view to another relation, tracked down to index/column level.
   message Reference {

pkg/sql/schemachanger/scpb/uml/table.puml

@@ -550,6 +550,7 @@ object View
 View :  ViewID
 View : []UsesTypeIDs
 View : []UsesRelationIDs
+View : []UsesRoutineIDs
 View : []ForwardReferences
 View :  IsTemporary
 View :  IsMaterialized

pkg/sql/schemachanger/scplan/internal/opgen/opgen_view.go

@@ -45,6 +45,15 @@ func init() {
 						TypeIDs:                    this.UsesTypeIDs,
 					}
 				}),
+				emit(func(this *scpb.View) *scop.RemoveBackReferenceInFunctions {
+					if len(this.UsesRoutineIDs) == 0 {
+						return nil
+					}
+					return &scop.RemoveBackReferenceInFunctions{
+						BackReferencedDescriptorID: this.ViewID,
+						FunctionIDs:                this.UsesRoutineIDs,
+					}
+				}),
 				emit(func(this *scpb.View) *scop.RemoveBackReferencesInRelations {
 					if len(this.UsesRelationIDs) == 0 {
 						return nil