sql: fix REPLACE routine logic with multiple DEFAULT expressions

When we added support for DEFAULT expressions in routine signatures in
41cb974, we introduced a bug for
REPLACE functionality. Namely, we need to store the type-checked
serialized representation of the default expressions in the proto, so
the expressions in the existing Overload must also have been
type-checked (so that we can find the correct signature in the proto to
update). However, we had the incorrect short-circuiting behavior when
type-checking existing default expressions where we'd stop once
a "signature change" is discovered. The fix is simple - we need to
type-check all default expressions separately from checking whether the
signature has changed.

Release note (bug fix): Previously, CockroachDB could encounter an
internal error when replacing (via CREATE OR REPLACE stmt)
a user-defined function or a stored procedure when multiple DEFAULT
expressions are included in the old signature. The bug has been present
since v24.2 (when support for DEFAULT expressions was added) and is now
fixed.

Author: yuzefovich

pkg/ccl/logictestccl/testdata/logic_test/procedure_params

@@ -880,4 +880,23 @@ $$ LANGUAGE PLpgSQL;
 statement ok
 DROP PROCEDURE p;
 
+# Regression test for not type-checking all default expressions when replacing
+# the procedure (#155859).
+statement ok
+CREATE PROCEDURE p (s STRING(1) DEFAULT NULL, d DECIMAL(1, 1) DEFAULT NULL) AS $$
+BEGIN
+  NULL;
+END;
+$$ LANGUAGE PLpgSQL;
+
+statement ok
+CREATE OR REPLACE PROCEDURE p (s STRING(2) DEFAULT NULL, d DECIMAL(2, 2) DEFAULT NULL) AS $$
+BEGIN
+  NULL;
+END;
+$$ LANGUAGE PLpgSQL;
+
+statement ok
+DROP PROCEDURE p;
+
 subtest end

pkg/ccl/logictestccl/testdata/logic_test/udf_params

@@ -1048,4 +1048,23 @@ $$ LANGUAGE PLpgSQL;
 statement ok
 DROP PROCEDURE p;
 
+# Regression test for not type-checking all default expressions when replacing
+# the UDF (#155859).
+statement ok
+CREATE FUNCTION f (s STRING(1) DEFAULT NULL, d DECIMAL(1, 1) DEFAULT NULL) RETURNS VOID AS $$
+BEGIN
+  NULL;
+END;
+$$ LANGUAGE PLpgSQL;
+
+statement ok
+CREATE OR REPLACE FUNCTION f (s STRING(2) DEFAULT NULL, d DECIMAL(2, 2) DEFAULT NULL) RETURNS VOID AS $$
+BEGIN
+  NULL;
+END;
+$$ LANGUAGE PLpgSQL;
+
+statement ok
+DROP FUNCTION f;
+
 subtest end

pkg/sql/create_function.go

@@ -309,20 +309,26 @@ func (n *createFunctionNode) replaceFunction(
 		return err
 	}
 
+	// We allow two types of "signature changes":
+	// - reordering OUT parameters in respect to input ones, and
+	// - changing the DEFAULT expression.
 	signatureChanged := len(existing.OutParamOrdinals) != len(outParamOrdinals) ||
 		len(existing.DefaultExprs) != len(defaultExprs)
 	for i := 0; !signatureChanged && i < len(outParamOrdinals); i++ {
 		signatureChanged = existing.OutParamOrdinals[i] != outParamOrdinals[i] ||
 			!existing.OutParamTypes.GetAt(i).Equivalent(outParamTypes[i])
 	}
-	for i := 0; !signatureChanged && i < len(defaultExprs); i++ {
-		typ := existing.Types.GetAt(i + existing.Types.Length() - len(defaultExprs))
-		// Update the overload to store the type-checked expression since this
-		// is what is stored in the FunctionSignature proto.
+	// Additionally, we must type-check all DEFAULT expressions since we need to
+	// store the type-checked serialized form in the FunctionSignature proto.
+	// (This is also assumed in ReplaceOverload.)
+	for i := 0; i < len(existing.DefaultExprs); i++ {
+		typ := existing.Types.GetAt(i + existing.Types.Length() - len(existing.DefaultExprs))
 		existing.DefaultExprs[i], err = tree.TypeCheck(params.ctx, existing.DefaultExprs[i], params.p.SemaCtx(), typ)
 		if err != nil {
 			return err
 		}
+	}
+	for i := 0; !signatureChanged && i < len(defaultExprs); i++ {
 		signatureChanged = tree.Serialize(existing.DefaultExprs[i]) != defaultExprs[i]
 	}
 	if signatureChanged {

pkg/sql/logictest/testdata/logic_test/procedure_params

@@ -766,4 +766,19 @@ DROP PROCEDURE p;
 statement ok
 DROP SEQUENCE seq;
 
+# Regression test for not type-checking all default expressions when replacing
+# the procedure (#155859).
+statement ok
+CREATE PROCEDURE p (s STRING(1) DEFAULT NULL, d DECIMAL(1, 1) DEFAULT NULL) AS $$
+  SELECT 1;
+$$ LANGUAGE SQL;
+
+statement ok
+CREATE OR REPLACE PROCEDURE p (s STRING(2) DEFAULT NULL, d DECIMAL(2, 2) DEFAULT NULL) AS $$
+  SELECT 1;
+$$ LANGUAGE SQL;
+
+statement ok
+DROP PROCEDURE p;
+
 subtest end

pkg/sql/logictest/testdata/logic_test/udf_params

@@ -914,4 +914,19 @@ DROP FUNCTION f;
 statement ok
 DROP SEQUENCE seq;
 
+# Regression test for not type-checking all default expressions when replacing
+# the UDF (#155859).
+statement ok
+CREATE FUNCTION f (s STRING(1) DEFAULT NULL, d DECIMAL(1, 1) DEFAULT NULL) RETURNS INT AS $$
+  SELECT 1;
+$$ LANGUAGE SQL;
+
+statement ok
+CREATE OR REPLACE FUNCTION f (s STRING(2) DEFAULT NULL, d DECIMAL(2, 2) DEFAULT NULL) RETURNS INT AS $$
+  SELECT 1;
+$$ LANGUAGE SQL;
+
+statement ok
+DROP FUNCTION f;
+
 subtest end