Merge #119616

119616: opt: correctly reconcile column definition list with RECORD-returning UDFs r=DrewKimball a=DrewKimball

#### sql: remove usages of `types.IsRecordType`

Previously, the `types.IsRecordType` function was used in different
contexts (with vs without OUT-params, function params vs return type).
This made it difficult to determine whether a particular usage was
correct, and led to a few bugs in cases where additional checks
were necessary.

This commit replaces usages of `types.IsRecordType` with either:
1. `typ.Identical(types.AnyTuple)`, or
2. `typ.Oid() == oid.T_record`

The former should be used for a RECORD-returning routine with no
OUT-parameters, as well as for a RECORD-typed variable. The latter
should be used to match either a RECORD-returning routine, or one
with multiple OUT-parameters.

Informs #114846

Release note: None

#### optbuilder: use actual arg types when building routine with wildcard types

Previously, we would always pass the static parameter types when
building the routine. However, in some cases the static type is
a wildcard, so we actually need to use the actual argument type. Note
that always using the actual argument type can be incorrect (e.g. we'd
lose the tuple labels present in the static type).

Release note: None

#### opt: refactor optbuild paths for routines and generator functions

This commit heavily refactors the type-handling logic for routines and
generator functions. The hope is to make the code more readable, and also
make the changes in the next commit easier.

Informs #114846

Release note: None

#### opt: add assignment casts for UDFs used as a data source

Previously, attempting to use a RECORD-returning UDF as a data source
(e.g. `SELECT * FROM` syntax) would result in an internal error if the
column definition list types didn't match the columns of the last
statement. This commit fixes that by adding validation that the types
are either identical or can be assignment-casted, and adding assignment
casts if necessary.

Fixes #114846
Fixes #113186

Release note (bug fix): Fixed a bug that could cause an internal error of
the form `invalid datum type given: ..., expected ...` when a RECORD-returning
UDF used as a data source was supplied a column definition list with
mismatched types. This bug has existed since v23.1.0.

#### opt/optbuilder: check for coercibility instead of tuple types

This commit changes the handling of tuple-returning routines to mirror
that of postgres. In particular, when the routine return type is a tuple,
postgres first attempts to coerce result columns to the return type of the
routine. Only if that attempt fails, postgres wraps the result column in
a tuple, and again attempts the coercion. This change affects the handling
of routines that return (for example) a single composite-typed column. For
example, the following two logic tests should produce the same result:
```
statement ok
CREATE TYPE two_typ AS (x INT, y INT);
CREATE FUNCTION f() RETURNS two_typ LANGUAGE SQL AS $$ SELECT 1, 2; $$;

query T
SELECT f();
----
(1,2)
```
vs
```
statement ok
CREATE TYPE two_typ AS (x INT, y INT);
CREATE FUNCTION f() RETURNS two_typ LANGUAGE SQL AS $$ SELECT ROW(1, 2); $$;

query T
SELECT f();
----
(1,2)
```
There is not release note, since this shouldn't affect versions prior to 24.1.

Fixes #120942

Release note: None

Co-authored-by: Drew Kimball <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>

pkg/ccl/logictestccl/testdata/logic_test/plpgsql_record

@@ -80,11 +80,10 @@ CREATE OR REPLACE FUNCTION f() RETURNS RECORD AS $$
   END
 $$ LANGUAGE PLpgSQL;
 
-# TODO(drewk): Postgres returns NULL, not a tuple with a NULL element.
 query T
 SELECT f();
 ----
-()
+NULL
 
 statement ok
 CREATE OR REPLACE FUNCTION f() RETURNS RECORD AS $$
@@ -356,4 +355,97 @@ CREATE OR REPLACE FUNCTION f(n INT) RETURNS RECORD AS $$
   END
 $$ LANGUAGE PLpgSQL;
 
+# Test errors related to a UDF called with a column-definition list.
+subtest column_definition_errors
+
+statement ok
+CREATE TYPE one_typ AS (x INT);
+CREATE TYPE foo_typ AS (x INT, y INT);
+CREATE TYPE bar_typ AS (x INT, y INT);
+
+# Column-definition list cannot be used with a composite UDT.
+statement ok
+DROP FUNCTION f(INT);
+DROP FUNCTION f;
+CREATE FUNCTION f() RETURNS foo_typ LANGUAGE PLpgSQL AS $$ BEGIN RETURN ROW(1, 2); END $$;
+
+statement error pgcode 42601 pq: a column definition list is redundant for a function returning a named composite type
+SELECT * FROM f() AS g(bar bar_typ);
+
+# Column-definition list cannot be used with a scalar type.
+statement ok
+DROP FUNCTION f;
+CREATE FUNCTION f() RETURNS INT LANGUAGE PLpgSQL AS $$ BEGIN RETURN 1; END $$;
+
+statement error pgcode 42601 pq: a column definition list is only allowed for functions returning \"record\"
+SELECT * FROM f() AS g(bar FLOAT);
+
+# Column-definition list cannot be used with OUT-parameters.
+statement ok
+DROP FUNCTION f;
+CREATE FUNCTION f(OUT x INT, OUT y INT) RETURNS RECORD LANGUAGE PLpgSQL AS $$ BEGIN x := 1; y := 2; RETURN; END $$;
+
+statement error pgcode 42601 pq: a column definition list is redundant for a function with OUT parameters
+SELECT * FROM f() AS g(bar bar_typ);
+
+# The number of result columns must match the number of entries in the column
+# definition list.
+statement ok
+DROP FUNCTION f;
+CREATE FUNCTION f() RETURNS RECORD LANGUAGE PLpgSQL AS $$ BEGIN RETURN (1, 2); END $$;
+
+statement error pgcode 42804 pq: returned record type does not match expected record type
+SELECT * FROM f() AS g(bar INT);
+
+statement error pgcode 42804 pq: returned record type does not match expected record type
+SELECT * FROM f() AS g(foo INT, bar INT, baz INT);
+
+# RECORD-returning UDF requires a column-definition list.
+statement ok
+DROP FUNCTION f;
+CREATE FUNCTION f() RETURNS RECORD LANGUAGE PLpgSQL AS $$ BEGIN RETURN ROW(1, 2); END $$;
+
+statement error pgcode 42601 pq: a column definition list is required for functions returning \"record\"
+SELECT * FROM f();
+
+# A column alias list is insufficient.
+statement error pgcode 42601 pq: a column definition list is required for functions returning \"record\"
+SELECT * FROM f() AS g(bar, baz);
+
+# The result column(s) must be assignment-cast compatible with the
+# column-definition list.
+statement ok
+DROP FUNCTION f;
+
+# Note: postgres doesn't throw this error until executing the UDF.
+statement error pgcode 42804 pq: cannot return non-composite value from function returning composite type
+CREATE FUNCTION f() RETURNS RECORD LANGUAGE PLpgSQL AS $$ BEGIN RETURN True; END $$;
+
+statement ok
+CREATE FUNCTION f() RETURNS RECORD LANGUAGE PLpgSQL AS $$ BEGIN RETURN ROW(True); END $$;
+
+statement error pgcode 42P13 pq: return type mismatch in function declared to return record
+SELECT * FROM f() AS g(bar one_typ);
+
+subtest regression_113186
+
+# Check whether the actual function result types are identical to the types in
+# the column definition, and correctly resolve differences.
+# Regression test for #113186.
+statement ok
+CREATE FUNCTION f113186() RETURNS RECORD LANGUAGE PLpgSQL AS $$ BEGIN RETURN ROW(1.99); END; $$;
+
+query R
+SELECT * FROM f113186() AS foo(x FLOAT);
+----
+1.99
+
+query I
+SELECT * FROM f113186() AS foo(x INT);
+----
+2
+
+statement error pgcode 42P13 pq: return type mismatch in function declared to return record
+SELECT * FROM f113186() AS foo(x TIMESTAMP);
+
 subtest end

pkg/ccl/logictestccl/testdata/logic_test/udf_params

@@ -67,29 +67,28 @@ SELECT f(3);
 ----
 NOTICE: 2
 
-# TODO(120942): figure out why this causes an internal error.
-# query II colnames
-# SELECT * FROM f(3);
-# ----
-# param1 param2
-# 3 2
-
-# query T noticetrace
-# SELECT * FROM f(3);
-# ----
-# NOTICE: 2
-
-# query I colnames
-# SELECT param1 FROM f(3);
-# ----
-# param1
-# 3
-
-# query I colnames
-# SELECT param2 FROM f(3);
-# ----
-# param2
-# 2
+query II colnames
+SELECT * FROM f(3);
+----
+param1 param2
+3 2
+
+query T noticetrace
+SELECT * FROM f(3);
+----
+NOTICE: 2
+
+query I colnames
+SELECT param1 FROM f(3);
+----
+param1
+3
+
+query I colnames
+SELECT param2 FROM f(3);
+----
+param2
+2
 
 statement ok
 DROP FUNCTION f;
@@ -127,19 +126,18 @@ NOTICE: 1 <NULL>
 NOTICE: 3 <NULL>
 NOTICE: 3 4
 
-# TODO(120942): figure this out.
-# query II colnames
-# SELECT * FROM f(1);
-# ----
-# param1 param2
-# 3 4
-
-# query T noticetrace
-# SELECT * FROM f(1);
-# ----
-# NOTICE: 1 <NULL>
-# NOTICE: 3 <NULL>
-# NOTICE: 3 4
+query II colnames
+SELECT * FROM f(1);
+----
+param1 param2
+3 4
+
+query T noticetrace
+SELECT * FROM f(1);
+----
+NOTICE: 1 <NULL>
+NOTICE: 3 <NULL>
+NOTICE: 3 4
 
 statement ok
 DROP FUNCTION f;

pkg/internal/sqlsmith/plpgsql.go

@@ -60,7 +60,7 @@ func (s *Smither) makePLpgSQLDeclarations(
 			varName = s.name("decl")
 		}
 		varTyp := s.randType()
-		for types.IsRecordType(varTyp) || varTyp.Family() == types.CollatedStringFamily {
+		for varTyp.Identical(types.AnyTuple) || varTyp.Family() == types.CollatedStringFamily {
 			// TODO(#114874): allow record types here when they are supported.
 			// TODO(#105245): allow collated strings when they are supported.
 			varTyp = s.randType()

pkg/internal/sqlsmith/relational.go

@@ -960,7 +960,7 @@ func (s *Smither) makeCreateFunc() (cf *tree.CreateRoutine, ok bool) {
 		// TODO(#105713): lift the RECORD-type restriction.
 		ptyp := s.randType()
 		for ptyp.Family() == types.CollatedStringFamily ||
-			(lang == tree.RoutineLangPLpgSQL && types.IsRecordType(ptyp)) {
+			(lang == tree.RoutineLangPLpgSQL && ptyp.Identical(types.AnyTuple)) {
 			ptyp = s.randType()
 		}
 		pname := fmt.Sprintf("p%d", i)

pkg/sql/catalog/funcdesc/func_desc.go

@@ -809,9 +809,7 @@ func (desc *immutable) ToOverload() (ret *tree.Overload, err error) {
 		ret.RoutineParams = append(ret.RoutineParams, routineParam)
 	}
 	ret.ReturnType = tree.FixedReturnType(desc.ReturnType.Type)
-	// TODO(yuzefovich): we should not be setting ReturnsRecordType to 'true'
-	// when the return type is based on output parameters.
-	ret.ReturnsRecordType = types.IsRecordType(desc.ReturnType.Type)
+	ret.ReturnsRecordType = desc.ReturnType.Type.Identical(types.AnyTuple)
 	ret.Types = signatureTypes
 	ret.Volatility, err = desc.getOverloadVolatility()
 	if err != nil {

pkg/sql/logictest/testdata/logic_test/procedure

@@ -405,6 +405,8 @@ CREATE PROCEDURE pv() AS 'SELECT 1' STRICT LANGUAGE SQL;
 
 subtest end
 
+subtest not_proc
+
 statement error pgcode 42809 sum is not a procedure
 CALL sum(1);
 
@@ -420,6 +422,8 @@ SELECT oid FROM pg_proc WHERE proname = 'count_rows';
 statement error pgcode 42809 count_rows is not a procedure
 CALL [ FUNCTION $funcOID ] ();
 
+subtest nested_call
+
 statement ok
 CREATE PROCEDURE p_inner(OUT param INTEGER) AS $$ SELECT 1; $$ LANGUAGE SQL;
 
@@ -433,3 +437,86 @@ CREATE PROCEDURE p_outer(OUT param INTEGER) AS $$ CALL p_inner(NULL); $$ LANGUAG
 
 statement ok
 DROP PROCEDURE p_inner;
+
+# Test type-coercion rules for composite return types.
+subtest return_tuple
+
+statement ok
+CREATE TYPE one_typ AS (x INT);
+CREATE TYPE two_typ AS (x INT, y INT);
+
+statement ok
+DROP PROCEDURE p(INT);
+DROP PROCEDURE p;
+
+# Test a procedure returning a composite type with one element.
+statement error pgcode 42P13 pq: return type mismatch in function declared to return record
+CREATE PROCEDURE p(OUT foo one_typ) LANGUAGE SQL AS $$ SELECT 1; $$;
+
+statement ok
+CREATE PROCEDURE p(OUT foo one_typ) LANGUAGE SQL AS $$ SELECT ROW(1); $$;
+
+query T
+CALL p(NULL);
+----
+(1)
+
+statement ok
+DROP PROCEDURE p;
+CREATE PROCEDURE p(OUT foo one_typ) LANGUAGE SQL AS $$ SELECT ROW(ROW(1)); $$;
+
+query T
+CALL p(NULL);
+----
+(1)
+
+statement ok
+DROP PROCEDURE p;
+
+# Test a procedure returning a composite type with two elements.
+statement error pgcode 42P13 pq: return type mismatch in function declared to return record
+CREATE PROCEDURE p(OUT foo two_typ) LANGUAGE SQL AS $$ SELECT 1, 2; $$;
+
+statement ok
+CREATE PROCEDURE p(OUT foo two_typ) LANGUAGE SQL AS $$ SELECT ROW(1, 2); $$;
+
+query T
+CALL p(NULL);
+----
+(1,2)
+
+statement ok
+DROP PROCEDURE p;
+CREATE PROCEDURE p(OUT foo two_typ) LANGUAGE SQL AS $$ SELECT ROW(ROW(1, 2)); $$;
+
+query T
+CALL p(NULL);
+----
+(1,2)
+
+# Test a procedure with two OUT-parameters.
+statement ok
+DROP PROCEDURE p;
+CREATE PROCEDURE p(OUT x INT, OUT y INT) LANGUAGE SQL AS $$ SELECT 1, 2; $$;
+
+query II
+CALL p(NULL, NULL);
+----
+1  2
+
+statement ok
+DROP PROCEDURE p;
+CREATE PROCEDURE p(OUT x INT, OUT y INT) LANGUAGE SQL AS $$ SELECT ROW(1, 2); $$;
+
+query II
+CALL p(NULL, NULL);
+----
+1  2
+
+statement ok
+DROP PROCEDURE p;
+
+statement error pgcode 42P13 pq: return type mismatch in function declared to return record
+CREATE PROCEDURE p(OUT x INT, OUT y INT) LANGUAGE SQL AS $$ SELECT ROW(ROW(1, 2)); $$;
+
+subtest end

pkg/sql/logictest/testdata/logic_test/procedure_params

@@ -499,12 +499,11 @@ CREATE TYPE typ AS (a INT, b INT);
 statement ok
 CREATE PROCEDURE p_udt(OUT typ) AS $$ SELECT (1, 2); $$ LANGUAGE SQL;
 
-# TODO(120942): this currently results in an internal error.
-# query T colnames
-# CALL p_udt(NULL);
-# ----
-# column1
-# (1,2)
+query T colnames
+CALL p_udt(NULL);
+----
+column1
+(1,2)
 
 statement error pgcode 2BP01 cannot drop type "typ" because other objects \(\[test.public.p_udt\]\) still depend on it
 DROP TYPE typ;