opt: check UDFs when checking metadata dependencies

This patch ensures that the metadata dependency-checking tracks user-defined
functions. This ensures that a cached query with a UDF reference will be
invalidated when the UDF is altered or dropped.

Fixes cockroachdb#93082

Release note (bug fix): Fixed a bug that could prevent a cached query
from being invalidated when a UDF referenced by that query was altered
or dropped.

pkg/sql/catalog/funcdesc/func_desc.go

@@ -539,6 +539,7 @@ func (desc *immutable) ToOverload() (ret *tree.Overload, err error) {
 		ReturnSet:  desc.ReturnType.ReturnSet,
 		Body:       desc.FunctionBody,
 		IsUDF:      true,
+		Version:    uint64(desc.Version),
 	}
 
 	argTypes := make(tree.ParamTypes, 0, len(desc.Params))

pkg/sql/logictest/testdata/logic_test/udf

@@ -2892,3 +2892,35 @@ SELECT f95240(a) FROM t95240
 ----
 33
 NULL
+
+# Regression test for #93082 - don't reuse a cached query with a UDF if the UDF
+# has been altered or dropped.
+subtest regression_93082
+
+statement ok
+CREATE FUNCTION fn(a INT) RETURNS INT LANGUAGE SQL AS 'SELECT a';
+
+query I
+SELECT fn(1);
+----
+1
+
+statement ok
+DROP FUNCTION fn;
+
+statement error pq: unknown function: fn\(\): function undefined
+SELECT fn(1);
+
+statement ok
+CREATE FUNCTION fn(a INT) RETURNS INT LANGUAGE SQL AS 'SELECT a';
+
+query I
+SELECT fn(1);
+----
+1
+
+statement ok
+ALTER FUNCTION fn RENAME TO fn2;
+
+statement error pq: unknown function: fn\(\): function undefined
+SELECT fn(1);

pkg/sql/opt/BUILD.bazel

@@ -25,6 +25,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/server/telemetry",
+        "//pkg/sql/catalog",
         "//pkg/sql/catalog/catpb",
         "//pkg/sql/catalog/colinfo",
         "//pkg/sql/catalog/descpb",

pkg/sql/opt/metadata.go

@@ -16,6 +16,7 @@ import (
 	"math/bits"
 	"strings"
 
+	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/multiregion"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/cat"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
@@ -101,6 +102,10 @@ type Metadata struct {
 	userDefinedTypes      map[oid.Oid]struct{}
 	userDefinedTypesSlice []*types.T
 
+	// userDefinedFunctions contains all user defined functions present in the
+	// query.
+	userDefinedFunctions map[*tree.Overload]struct{}
+
 	// deps stores information about all data source objects depended on by the
 	// query, as well as the privileges required to access them. The objects are
 	// deduplicated: any name/object pair shows up at most once.
@@ -292,17 +297,17 @@ func (md *Metadata) AddDependency(name MDDepName, ds cat.DataSource, priv privil
 // perform KV operations on behalf of the transaction associated with the
 // provided catalog, and those errors are required to be propagated.
 func (md *Metadata) CheckDependencies(
-	ctx context.Context, catalog cat.Catalog,
+	ctx context.Context, optCatalog cat.Catalog,
 ) (upToDate bool, err error) {
 	for i := range md.deps {
 		name := &md.deps[i].name
 		var toCheck cat.DataSource
 		var err error
 		if name.byID != 0 {
-			toCheck, _, err = catalog.ResolveDataSourceByID(ctx, cat.Flags{}, name.byID)
+			toCheck, _, err = optCatalog.ResolveDataSourceByID(ctx, cat.Flags{}, name.byID)
 		} else {
 			// Resolve data source object.
-			toCheck, _, err = catalog.ResolveDataSource(ctx, cat.Flags{}, &name.byName)
+			toCheck, _, err = optCatalog.ResolveDataSource(ctx, cat.Flags{}, &name.byName)
 		}
 		if err != nil {
 			return false, err
@@ -321,7 +326,7 @@ func (md *Metadata) CheckDependencies(
 			// privileges do not need to be checked). Ignore the "zero privilege".
 			priv := privilege.Kind(bits.TrailingZeros32(uint32(privs)))
 			if priv != 0 {
-				if err := catalog.CheckPrivilege(ctx, toCheck, priv); err != nil {
+				if err := optCatalog.CheckPrivilege(ctx, toCheck, priv); err != nil {
 					return false, err
 				}
 			}
@@ -332,7 +337,7 @@ func (md *Metadata) CheckDependencies(
 	}
 	// Check that all of the user defined types present have not changed.
 	for _, typ := range md.AllUserDefinedTypes() {
-		toCheck, err := catalog.ResolveTypeByOID(ctx, typ.Oid())
+		toCheck, err := optCatalog.ResolveTypeByOID(ctx, typ.Oid())
 		if err != nil {
 			// Handle when the type no longer exists.
 			if pgerror.GetPGCode(err) == pgcode.UndefinedObject {
@@ -344,6 +349,21 @@ func (md *Metadata) CheckDependencies(
 			return false, nil
 		}
 	}
+	// Check that all of the user defined functions have not changed.
+	for fun := range md.userDefinedFunctions {
+		_, toCheck, err := optCatalog.ResolveFunctionByOID(ctx, fun.Oid)
+		if err != nil {
+			// Handle when the function no longer exists.
+			if pgerror.GetPGCode(err) == pgcode.UndefinedObject ||
+				errors.Is(err, catalog.ErrDescriptorDropped) {
+				return false, nil
+			}
+			return false, err
+		}
+		if fun.Version != toCheck.Version {
+			return false, nil
+		}
+	}
 	return true, nil
 }
 
@@ -378,6 +398,18 @@ func (md *Metadata) AllUserDefinedTypes() []*types.T {
 	return md.userDefinedTypesSlice
 }
 
+// AddUserDefinedFunc adds a user defined function overload to the metadata for
+// this query.
+func (md *Metadata) AddUserDefinedFunc(fun *tree.Overload) {
+	if !fun.IsUDF {
+		return
+	}
+	if md.userDefinedFunctions == nil {
+		md.userDefinedFunctions = make(map[*tree.Overload]struct{})
+	}
+	md.userDefinedFunctions[fun] = struct{}{}
+}
+
 // AddTable indexes a new reference to a table within the query. Separate
 // references to the same table are assigned different table ids (e.g.  in a
 // self-join query). All columns are added to the metadata. If mutation columns

pkg/sql/opt/optbuilder/scalar.go

@@ -619,6 +619,7 @@ func (b *Builder) buildUDF(
 	colRefs *opt.ColSet,
 ) (out opt.ScalarExpr) {
 	o := f.ResolvedOverload()
+	b.factory.Metadata().AddUserDefinedFunc(o)
 
 	// Build the argument expressions.
 	var args memo.ScalarListExpr

pkg/sql/sem/tree/overload.go

@@ -191,6 +191,9 @@ type Overload struct {
 	// ReturnSet is set to true when a user-defined function is defined to return
 	// a set of values.
 	ReturnSet bool
+	// Version is the descriptor version of the descriptor used to construct
+	// this version of the function overload. Only used for UDFs.
+	Version uint64
 }
 
 // params implements the overloadImpl interface.