diff --git a/pkg/ccl/logictestccl/testdata/logic_test/plpgsql_record b/pkg/ccl/logictestccl/testdata/logic_test/plpgsql_record index f8a803d1885a..fdee824198f2 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/plpgsql_record +++ b/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 diff --git a/pkg/ccl/logictestccl/testdata/logic_test/udf_params b/pkg/ccl/logictestccl/testdata/logic_test/udf_params index 85d793439cc5..a416691811ff 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/udf_params +++ b/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 NOTICE: 3 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 -# NOTICE: 3 -# NOTICE: 3 4 +query II colnames +SELECT * FROM f(1); +---- +param1 param2 +3 4 + +query T noticetrace +SELECT * FROM f(1); +---- +NOTICE: 1 +NOTICE: 3 +NOTICE: 3 4 statement ok DROP FUNCTION f; diff --git a/pkg/internal/sqlsmith/plpgsql.go b/pkg/internal/sqlsmith/plpgsql.go index 0d35b72f5f4c..47fcbae7f8dd 100644 --- a/pkg/internal/sqlsmith/plpgsql.go +++ b/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() diff --git a/pkg/internal/sqlsmith/relational.go b/pkg/internal/sqlsmith/relational.go index 825928a4a1d8..fd89a4b3bdee 100644 --- a/pkg/internal/sqlsmith/relational.go +++ b/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) diff --git a/pkg/sql/catalog/funcdesc/func_desc.go b/pkg/sql/catalog/funcdesc/func_desc.go index 440f0b0721a2..5fb1c2bd8b09 100644 --- a/pkg/sql/catalog/funcdesc/func_desc.go +++ b/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 { diff --git a/pkg/sql/logictest/testdata/logic_test/procedure b/pkg/sql/logictest/testdata/logic_test/procedure index 7f48b38f765a..c05a196cf8bc 100644 --- a/pkg/sql/logictest/testdata/logic_test/procedure +++ b/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 diff --git a/pkg/sql/logictest/testdata/logic_test/procedure_params b/pkg/sql/logictest/testdata/logic_test/procedure_params index 3edfa4cbe926..d33f69b45396 100644 --- a/pkg/sql/logictest/testdata/logic_test/procedure_params +++ b/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; diff --git a/pkg/sql/logictest/testdata/logic_test/udf b/pkg/sql/logictest/testdata/logic_test/udf index b2fcbc150ef7..c4609e393c94 100644 --- a/pkg/sql/logictest/testdata/logic_test/udf +++ b/pkg/sql/logictest/testdata/logic_test/udf @@ -779,11 +779,124 @@ FROM purchase 1.00 BHD 10.00 GBP -subtest end +subtest regression_113186 statement ok CREATE FUNCTION f113186() RETURNS RECORD AS $$ SELECT 1.99; $$ LANGUAGE SQL; -# Until #113186 is resolved, the internal error is expected. -statement error pgcode XX000 internal error: invalid datum type given: DECIMAL, expected INT8 +query I SELECT * FROM f113186() AS foo(x INT); +---- +2 + +# 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); + +# Test a function returning a composite type with one element. +statement ok +DROP FUNCTION f; +CREATE FUNCTION f() RETURNS one_typ LANGUAGE SQL AS $$ SELECT 1; $$; + +query T +SELECT f(); +---- +(1) + +query I +SELECT * FROM f(); +---- +1 + +statement ok +DROP FUNCTION f; +CREATE FUNCTION f() RETURNS one_typ LANGUAGE SQL AS $$ SELECT ROW(1); $$; + +query T +SELECT f(); +---- +(1) + +query I +SELECT * FROM f(); +---- +1 + +statement ok +DROP FUNCTION f; + +statement error pgcode 42P13 pq: return type mismatch in function declared to return one_typ +CREATE FUNCTION f() RETURNS one_typ LANGUAGE SQL AS $$ SELECT ROW(ROW(1)); $$; + +# Test a function returning a composite type with two elements. +statement ok +CREATE FUNCTION f() RETURNS two_typ LANGUAGE SQL AS $$ SELECT 1, 2; $$; + +query T +SELECT f(); +---- +(1,2) + +query II +SELECT * FROM f(); +---- +1 2 + +statement ok +DROP FUNCTION f; +CREATE FUNCTION f() RETURNS two_typ LANGUAGE SQL AS $$ SELECT ROW(1, 2); $$; + +query T +SELECT f(); +---- +(1,2) + +query II +SELECT * FROM f(); +---- +1 2 + +statement ok +DROP FUNCTION f; + +statement error pgcode 42P13 pq: return type mismatch in function declared to return two_typ +CREATE FUNCTION f() RETURNS two_typ LANGUAGE SQL AS $$ SELECT ROW(ROW(1, 2)); $$; + +# Test a function with two OUT-parameters. +statement ok +CREATE FUNCTION f(OUT x INT, OUT y INT) LANGUAGE SQL AS $$ SELECT 1, 2; $$; + +query T +SELECT f(); +---- +(1,2) + +query II +SELECT * FROM f(); +---- +1 2 + +statement ok +DROP FUNCTION f; +CREATE FUNCTION f(OUT x INT, OUT y INT) LANGUAGE SQL AS $$ SELECT ROW(1, 2); $$; + +query T +SELECT f(); +---- +(1,2) + +query II +SELECT * FROM f(); +---- +1 2 + +statement ok +DROP FUNCTION f; + +statement error pgcode 42P13 pq: return type mismatch in function declared to return record +CREATE FUNCTION f(OUT x INT, OUT y INT) LANGUAGE SQL AS $$ SELECT ROW(ROW(1, 2)); $$; + +subtest end diff --git a/pkg/sql/logictest/testdata/logic_test/udf_record b/pkg/sql/logictest/testdata/logic_test/udf_record index 88e3b6c4eb89..4922e035cdcb 100644 --- a/pkg/sql/logictest/testdata/logic_test/udf_record +++ b/pkg/sql/logictest/testdata/logic_test/udf_record @@ -211,7 +211,7 @@ $$ $$ LANGUAGE SQL; query T -SELECT * FROM f_udt() AS foo(u udt); +SELECT * FROM f_udt(); ---- a @@ -465,20 +465,24 @@ CREATE FUNCTION imp() RETURNS imp LANGUAGE SQL AS $$ SELECT k, a, b FROM imp $$ -query TBT -SELECT imp(), (1,2,'a')::imp = imp(), pg_typeof(imp()) +# TODO(#58252): pg_typeof(imp_tup_ordered()) is omitted because we get +# different results for different configurations. +query TB +SELECT imp(), (1,2,'a')::imp = imp() ---- -(1,2,a) true imp +(1,2,a) true statement ok CREATE FUNCTION imp_star() RETURNS imp LANGUAGE SQL AS $$ SELECT * FROM imp $$ -query TBT -SELECT imp_star(), (1,2,'a')::imp = imp_star(), pg_typeof(imp_star()) +# TODO(#58252): pg_typeof(imp_tup_ordered()) is omitted because we get +# different results for different configurations. +query TB +SELECT imp_star(), (1,2,'a')::imp = imp_star() ---- -(1,2,a) true imp +(1,2,a) true statement ok INSERT INTO imp VALUES (100, 200, 'z') @@ -488,8 +492,8 @@ CREATE FUNCTION imp_tup_ordered() RETURNS imp LANGUAGE SQL AS $$ SELECT (k, a, b) FROM imp ORDER BY b DESC $$ -# TODO(mgartner): pg_typeof(imp_tup_ordered()) is omitted because we get -# different results for different configurations, due to #58252. +# TODO(#58252): pg_typeof(imp_tup_ordered()) is omitted because we get +# different results for different configurations. query TB SELECT imp_tup_ordered(), (100,200,'z')::imp = imp_tup_ordered() ---- @@ -500,10 +504,12 @@ CREATE FUNCTION imp_ordered() RETURNS imp LANGUAGE SQL AS $$ SELECT k, a, b FROM imp ORDER BY b DESC $$ -query TBT -SELECT imp_ordered(), (100,200,'z')::imp = imp_ordered(), pg_typeof(imp_ordered()) +# TODO(#58252): pg_typeof(imp_ordered()) is omitted because we get +# different results for different configurations. +query TB +SELECT imp_ordered(), (100,200,'z')::imp = imp_ordered() ---- -(100,200,z) true imp +(100,200,z) true statement ok CREATE FUNCTION imp_identity(i imp) RETURNS imp LANGUAGE SQL AS $$ @@ -559,4 +565,105 @@ CREATE FUNCTION err(i imp) RETURNS INT LANGUAGE SQL AS $$ SELECT i $$ +# Test errors related to a UDF called with a column-definition list. +subtest column_definition_errors + +statement ok +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 +CREATE FUNCTION f() RETURNS foo_typ LANGUAGE SQL AS $$ SELECT ROW(1, 2); $$; + +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 SQL AS $$ SELECT 1; $$; + +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 SQL AS $$ SELECT ROW(1, 2); $$; + +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 SQL AS $$ SELECT 1, 2; $$; + +statement error pgcode 42804 pq: function return row and query-specified return row do not match +SELECT * FROM f() AS g(bar INT); + +statement error pgcode 42804 pq: function return row and query-specified return row do not match +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 SQL AS $$ SELECT ROW(1, 2); $$; + +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; +CREATE FUNCTION f() RETURNS RECORD LANGUAGE SQL AS $$ SELECT True; $$; + +statement error pgcode 42P13 pq: return type mismatch in function declared to return record +SELECT * FROM f() AS g(bar INT); + +subtest regression_113186 + +# Check whether the actual function result types are identical to the types in +# the column definition, and correctly resolve differences. +statement ok +CREATE FUNCTION f113186() RETURNS RECORD LANGUAGE SQL AS $$ SELECT 1.99; $$; + +query R colnames +SELECT * FROM f113186() AS foo(x FLOAT); +---- +x +1.99 + +query I colnames +SELECT * FROM f113186() AS foo(x INT); +---- +x +2 + +statement error pgcode 42P13 pq: return type mismatch in function declared to return record +SELECT * FROM f113186() AS foo(x TIMESTAMP); + +subtest regression_114846 + +# Function creation and execution should succeed without internal error due to +# mismatched types. +statement ok +CREATE FUNCTION array_to_set(ANYARRAY) RETURNS SETOF RECORD AS $$ + SELECT i AS "index", $1[i] AS "value" FROM generate_subscripts($1, 1) i; +$$ LANGUAGE SQL STRICT IMMUTABLE; + +query RT colnames,rowsort +SELECT * FROM array_to_set(ARRAY['one', 'two']) AS t(f1 NUMERIC(4,2), f2 TEXT); +---- +f1 f2 +1.00 one +2.00 two + subtest end diff --git a/pkg/sql/opt/exec/execbuilder/testdata/call b/pkg/sql/opt/exec/execbuilder/testdata/call index 58a9b945c57c..b318dec3fbb1 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/call +++ b/pkg/sql/opt/exec/execbuilder/testdata/call @@ -72,11 +72,11 @@ call │ ├── fd: ()-->(3) │ └── (2,) └── values - ├── columns: column5:5 + ├── columns: column1:4 ├── cardinality: [1 - 1] ├── stats: [rows=1] ├── key: () - ├── fd: ()-->(5) + ├── fd: ()-->(4) └── (NULL,) query T @@ -110,13 +110,13 @@ call │ └── tuple [type=tuple{int}] │ └── const: 2 [type=int] └── values - ├── columns: column5:5(void) + ├── columns: column1:4(unknown) ├── cardinality: [1 - 1] ├── stats: [rows=1] ├── key: () - ├── fd: ()-->(5) - └── tuple [type=tuple{void}] - └── null [type=void] + ├── fd: ()-->(4) + └── tuple [type=tuple{unknown}] + └── null [type=unknown] query T EXPLAIN (DISTSQL) CALL foo(3); diff --git a/pkg/sql/opt/exec/execbuilder/testdata/udf b/pkg/sql/opt/exec/execbuilder/testdata/udf index d806baa33b85..71d123fdcc48 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/udf +++ b/pkg/sql/opt/exec/execbuilder/testdata/udf @@ -362,3 +362,23 @@ vectorized: true columns: (f88259) size: 1 column, 1 row row 0, expr 0: f88259(333, 444) + +# Regression test for not using actual argument types (#114846). +statement ok +CREATE FUNCTION array_to_set(anyarray) RETURNS SETOF RECORD AS $$ + SELECT i AS "index", $1[i] AS "value" FROM generate_subscripts($1, 1) i +$$ LANGUAGE SQL STRICT IMMUTABLE; + +query T +EXPLAIN (VERBOSE, TYPES) SELECT * FROM array_to_set(array['one', 'two']) AS t(f1 numeric(4,2), f2 text); +---- +distribution: local +vectorized: true +· +• project set +│ columns: (f1 decimal, f2 string) +│ estimated row count: 1 +│ render 0: (array_to_set((ARRAY[('one')[string],('two')[string]])[string[]]))[tuple{decimal AS f1, string AS f2}] +│ +└── • emptyrow + columns: () diff --git a/pkg/sql/opt/memo/expr.go b/pkg/sql/opt/memo/expr.go index 8d5035c6abb3..3b2ce3a31cb3 100644 --- a/pkg/sql/opt/memo/expr.go +++ b/pkg/sql/opt/memo/expr.go @@ -705,7 +705,7 @@ type UDFDefinition struct { CalledOnNullInput bool // MultiColDataSource is true if the function may return multiple columns. - // This is only the case if the UDF returns a RECORD type and is used as a + // This is only the case if the UDF returns a composite type and is used as a // data source. MultiColDataSource bool diff --git a/pkg/sql/opt/optbuilder/create_function.go b/pkg/sql/opt/optbuilder/create_function.go index 8b660788a2cf..308fd9a19404 100644 --- a/pkg/sql/opt/optbuilder/create_function.go +++ b/pkg/sql/opt/optbuilder/create_function.go @@ -202,7 +202,7 @@ func (b *Builder) buildCreateFunction(cf *tree.CreateRoutine, inScope *scope) (o } // The parameter type must be supported by the current cluster version. checkUnsupportedType(b.ctx, b.semaCtx, typ) - if types.IsRecordType(typ) { + if typ.Identical(types.AnyTuple) { if language == tree.RoutineLangSQL { panic(pgerror.Newf(pgcode.InvalidFunctionDefinition, "SQL functions cannot have arguments of type record")) @@ -398,8 +398,7 @@ func (b *Builder) buildCreateFunction(cf *tree.CreateRoutine, inScope *scope) (o // TODO(mgartner): stmtScope.cols does not describe the result // columns of the statement. We should use physical.Presentation // instead. - isSQLProcedure := cf.IsProcedure && language == tree.RoutineLangSQL - err = validateReturnType(b.ctx, b.semaCtx, funcReturnType, stmtScope.cols, isSQLProcedure) + err = validateReturnType(b.ctx, b.semaCtx, funcReturnType, stmtScope.cols) if err != nil { panic(err) } @@ -449,11 +448,7 @@ func formatFuncBodyStmt( } func validateReturnType( - ctx context.Context, - semaCtx *tree.SemaContext, - expected *types.T, - cols []scopeColumn, - isSQLProcedure bool, + ctx context.Context, semaCtx *tree.SemaContext, expected *types.T, cols []scopeColumn, ) error { // The return type must be supported by the current cluster version. checkUnsupportedType(ctx, semaCtx, expected) @@ -478,30 +473,36 @@ func validateReturnType( // If return type is RECORD and the tuple content types unspecified by OUT // parameters, any column types are valid. This is the case when we have - // RETURNS RECORD without OUT params - we don't need to check the types + // RETURNS RECORD without OUT-params - we don't need to check the types // below. - if types.IsRecordType(expected) && types.IsWildcardTupleType(expected) { + if expected.Identical(types.AnyTuple) { return nil } if len(cols) == 1 { - typeToCheck := expected - if isSQLProcedure && types.IsRecordType(expected) && len(expected.TupleContents()) == 1 { - // For SQL procedures with output parameters we get a record type - // even with a single column. - typeToCheck = expected.TupleContents()[0] - } - if !typeToCheck.Equivalent(cols[0].typ) && - !cast.ValidCast(cols[0].typ, typeToCheck, cast.ContextAssignment) { - return pgerror.WithCandidateCode( - errors.WithDetailf( - errors.Newf("return type mismatch in function declared to return %s", expected.Name()), - "Actual return type is %s", cols[0].typ.Name(), - ), - pgcode.InvalidFunctionDefinition, - ) + if expected.Equivalent(cols[0].typ) || + cast.ValidCast(cols[0].typ, expected, cast.ContextAssignment) { + // Postgres allows UDFs to coerce a single result column directly to the + // return type. Stored procedures are not allowed to do this (see below). + return nil + } + if len(expected.TupleContents()) == 1 { + // The routine returns a composite type with one element. This is the case + // for a UDF with a composite return type, or a stored procedure with a + // single OUT-parameter. In either case, the column can be coerced to the + // tuple element type. + if expected.TupleContents()[0].Equivalent(cols[0].typ) || + cast.ValidCast(cols[0].typ, expected.TupleContents()[0], cast.ContextAssignment) { + return nil + } } - return nil + return pgerror.WithCandidateCode( + errors.WithDetailf( + errors.Newf("return type mismatch in function declared to return %s", expected.Name()), + "Actual return type is %s", cols[0].typ.Name(), + ), + pgcode.InvalidFunctionDefinition, + ) } // If the last statement return multiple columns, then the expected Family diff --git a/pkg/sql/opt/optbuilder/plpgsql.go b/pkg/sql/opt/optbuilder/plpgsql.go index ec9749939847..7f5a274303f9 100644 --- a/pkg/sql/opt/optbuilder/plpgsql.go +++ b/pkg/sql/opt/optbuilder/plpgsql.go @@ -337,7 +337,7 @@ func (b *plpgsqlBuilder) buildBlock(astBlock *ast.Block, s *scope) *scope { if err != nil { panic(err) } - if types.IsRecordType(typ) { + if typ.Identical(types.AnyTuple) { panic(recordVarErr) } b.addVariable(dec.Var, typ) @@ -360,7 +360,7 @@ func (b *plpgsqlBuilder) buildBlock(astBlock *ast.Block, s *scope) *scope { block.cursors[dec.Name] = *dec } } - if types.IsRecordType(b.returnType) && types.IsWildcardTupleType(b.returnType) { + if b.returnType.Identical(types.AnyTuple) { // For a RECORD-returning routine, infer the concrete type by examining the // RETURN statements. This has to happen after building the declaration // block because RETURN statements can reference declared variables. Only @@ -931,7 +931,7 @@ func (b *plpgsqlBuilder) buildPLpgSQLStatements(stmts []ast.Statement, s *scope) procTyp := proc.ResolvedType() colName := scopeColName("").WithMetadataName(b.makeIdentifier("stmt_call")) col := b.ob.synthesizeColumn(callScope, colName, procTyp, nil /* expr */, nil /* scalar */) - procScalar, _ := b.ob.buildRoutine(proc, def, callCon.s, callScope, b.colRefs) + procScalar := b.ob.buildRoutine(proc, def, callCon.s, callScope, b.colRefs) col.scalar = procScalar b.ob.constructProjectForScope(callCon.s, callScope) diff --git a/pkg/sql/opt/optbuilder/routine.go b/pkg/sql/opt/optbuilder/routine.go index 9dfa3d0ae2f3..447a83256fbd 100644 --- a/pkg/sql/opt/optbuilder/routine.go +++ b/pkg/sql/opt/optbuilder/routine.go @@ -24,7 +24,6 @@ import ( plpgsql "github.com/cockroachdb/cockroach/pkg/sql/plpgsql/parser" "github.com/cockroachdb/cockroach/pkg/sql/sem/cast" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" - "github.com/cockroachdb/cockroach/pkg/sql/sqlerrors" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" "github.com/cockroachdb/errors" @@ -62,12 +61,12 @@ func (b *Builder) buildUDF( } // Build the routine. - routine, isMultiColDataSource := b.buildRoutine(f, def, inScope, outScope, colRefs) + routine := b.buildRoutine(f, def, inScope, outScope, colRefs) // Synthesize an output columns if necessary. if outCol == nil { - if isMultiColDataSource { - return b.finishBuildGeneratorFunction(f, o, routine, inScope, outScope, outCol) + if b.insideDataSource && len(f.ResolvedType().TupleContents()) > 0 { + return b.finishBuildGeneratorFunction(f, routine, inScope, outScope, outCol) } if outScope != nil { outCol = b.synthesizeColumn(outScope, scopeColName(""), f.ResolvedType(), nil /* expr */, routine) @@ -129,7 +128,7 @@ func (b *Builder) buildProcedure(c *tree.Call, inScope *scope) *scope { } // Build the routine. - routine, _ := b.buildRoutine(proc, def, inScope, outScope, nil /* colRefs */) + routine := b.buildRoutine(proc, def, inScope, outScope, nil /* colRefs */) routine = b.finishBuildScalar(nil /* texpr */, routine, inScope, nil /* outScope */, nil /* outCol */) @@ -202,7 +201,7 @@ func (b *Builder) buildRoutine( def *tree.ResolvedFunctionDefinition, inScope, outScope *scope, colRefs *opt.ColSet, -) (out opt.ScalarExpr, isMultiColDataSource bool) { +) opt.ScalarExpr { o := f.ResolvedOverload() isProc := o.Type == tree.ProcedureRoutine invocationTypes := make([]*types.T, len(f.Exprs)) @@ -218,42 +217,18 @@ func (b *Builder) buildRoutine( // Validate that the return types match the original return types defined in // the function. Return types like user defined return types may change // since the function was first created. - rtyp := f.ResolvedType() - if rtyp.UserDefined() { + originalReturnType := f.ResolvedType() + if originalReturnType.UserDefined() { funcReturnType, err := tree.ResolveType(b.ctx, - &tree.OIDTypeReference{OID: rtyp.Oid()}, b.semaCtx.TypeResolver) + &tree.OIDTypeReference{OID: originalReturnType.Oid()}, b.semaCtx.TypeResolver) if err != nil { panic(err) } - if !funcReturnType.Identical(rtyp) { + if !funcReturnType.Identical(originalReturnType) { panic(pgerror.Newf( pgcode.InvalidFunctionDefinition, - "return type mismatch in function declared to return %s", rtyp.Name())) - } - } - // If returning a RECORD type, the function return type needs to be modified - // because when we first parse the CREATE FUNCTION, the RECORD is - // represented as a tuple with any types and execution requires the types to - // be concrete in order to decode them correctly. We can determine the types - // from the result columns or tuple of the last statement. - finishResolveType := func(lastStmtScope *scope) { - if types.IsWildcardTupleType(rtyp) { - if len(lastStmtScope.cols) == 1 && - lastStmtScope.cols[0].typ.Family() == types.TupleFamily { - // When the final statement returns a single tuple, we can use - // the tuple's types as the function return type. - f.SetTypeAnnotation(lastStmtScope.cols[0].typ) - } else { - // Get the types from the individual columns of the last - // statement. - tc := make([]*types.T, len(lastStmtScope.cols)) - tl := make([]string, len(lastStmtScope.cols)) - for i, col := range lastStmtScope.cols { - tc[i] = col.typ - tl[i] = col.name.MetadataName() - } - f.SetTypeAnnotation(types.MakeLabeledTuple(tc, tl)) - } + "return type mismatch in function declared to return %s", originalReturnType.Name(), + )) } } @@ -349,11 +324,22 @@ func (b *Builder) buildRoutine( panic(unimplemented.NewWithIssue(88947, "variadiac user-defined functions are not yet supported")) } + if len(paramTypes) != len(args) { + panic(errors.AssertionFailedf( + "different number of static parameters %d and actual arguments %d", len(paramTypes), len(args), + )) + } params = make(opt.ColList, len(paramTypes)) for i := range paramTypes { paramType := ¶mTypes[i] argColName := funcParamColName(tree.Name(paramType.Name), i) - col := b.synthesizeColumn(bodyScope, argColName, paramType.Typ, nil /* expr */, nil /* scalar */) + // Use the statically defined parameter type (unless it is a wildcard, in + // which case we use the actual argument type). + argType := paramType.Typ + if argType.IsWildcardType() { + argType = args[i].DataType() + } + col := b.synthesizeColumn(bodyScope, argColName, argType, nil /* expr */, nil /* scalar */) col.setParamOrd(i) params[i] = col.id } @@ -376,7 +362,6 @@ func (b *Builder) buildRoutine( b.trackSchemaDeps = false b.insideUDF = true isSetReturning := o.Class == tree.GeneratorClass - isMultiColDataSource = false // Build an expression for each statement in the function body. var body []memo.RelExpr @@ -395,7 +380,7 @@ func (b *Builder) buildRoutine( // not be executed. // TODO(mgartner): This will add some planning overhead for every // invocation of the function. Is there a more efficient way to do this? - if rtyp.Family() == types.VoidFamily { + if originalReturnType.Family() == types.VoidFamily { stmts = append(stmts, statements.Statement[tree.Statement]{ AST: &tree.Select{ Select: &tree.ValuesClause{ @@ -414,9 +399,9 @@ func (b *Builder) buildRoutine( // The last statement produces the output of the UDF. if i == len(stmts)-1 { - finishResolveType(stmtScope) - expr, physProps, isMultiColDataSource = - b.finishBuildLastStmt(stmtScope, bodyScope, isSetReturning, oldInsideDataSource, f) + expr, physProps = b.finishBuildLastStmt( + stmtScope, bodyScope, inScope, isSetReturning, oldInsideDataSource, f, + ) } body[i] = expr bodyProps[i] = physProps @@ -451,12 +436,12 @@ func (b *Builder) buildRoutine( var expr memo.RelExpr var physProps *physical.Required plBuilder := newPLpgSQLBuilder( - b, def.Name, stmt.AST.Label, colRefs, routineParams, rtyp, isProc, outScope, + b, def.Name, stmt.AST.Label, colRefs, routineParams, originalReturnType, isProc, outScope, ) stmtScope := plBuilder.buildRootBlock(stmt.AST, bodyScope, routineParams) - finishResolveType(stmtScope) - expr, physProps, isMultiColDataSource = - b.finishBuildLastStmt(stmtScope, bodyScope, isSetReturning, oldInsideDataSource, f) + expr, physProps = b.finishBuildLastStmt( + stmtScope, bodyScope, inScope, isSetReturning, oldInsideDataSource, f, + ) body = []memo.RelExpr{expr} bodyProps = []*physical.Required{physProps} if b.verboseTracing { @@ -466,16 +451,20 @@ func (b *Builder) buildRoutine( panic(errors.AssertionFailedf("unexpected language: %v", o.Language)) } + // NOTE: originalReturnType may not be up-to-date after the last statement is + // built, so we call f.ResolvedType() again here. + rTyp := f.ResolvedType() + multiColDataSource := len(rTyp.TupleContents()) > 0 && oldInsideDataSource routine := b.factory.ConstructUDFCall( args, &memo.UDFCallPrivate{ Def: &memo.UDFDefinition{ Name: def.Name, - Typ: f.ResolvedType(), + Typ: rTyp, Volatility: o.Volatility, SetReturning: isSetReturning, CalledOnNullInput: o.CalledOnNullInput, - MultiColDataSource: isMultiColDataSource, + MultiColDataSource: multiColDataSource, RoutineType: o.Type, RoutineLang: o.Language, Body: body, @@ -485,22 +474,30 @@ func (b *Builder) buildRoutine( }, }, ) - return routine, isMultiColDataSource + return routine } // finishBuildLastStmt manages the columns returned by the last statement of a -// UDF. Depending on the context and return type of the UDF, this may mean -// expanding a tuple into multiple columns, or combining multiple columns into -// a tuple. +// routine. Depending on the context and return type of the routine, this may +// mean expanding a tuple into multiple columns, or combining multiple columns +// into a tuple. // -// insideDataSource indicates whether the routine is used as a data source. It -// is passed in rather than using b.insideDataSource because b.insideDataSource -// is reset while building the body of the routine. +// finishBuildLastStmt also determines the final return type for the routine +// based on the last statement's result columns, and updates the type annotation +// for the FuncExpr accordingly. func (b *Builder) finishBuildLastStmt( - stmtScope *scope, bodyScope *scope, isSetReturning, insideDataSource bool, f *tree.FuncExpr, -) (expr memo.RelExpr, physProps *physical.Required, isMultiColDataSource bool) { + stmtScope, bodyScope, inScope *scope, isSetReturning, insideDataSource bool, f *tree.FuncExpr, +) (expr memo.RelExpr, physProps *physical.Required) { + // After this call to finalizeRoutineReturnType, the type annotation will + // reflect the final resolved type of the function. + // + // NOTE: the result columns of the last statement may not reflect this type + // until after the call to maybeAddRoutineAssignmentCasts. Therefore, the + // logic below must take care in distinguishing the resolved return type from + // the result column type(s). + b.finalizeRoutineReturnType(f, stmtScope, inScope, insideDataSource) expr, physProps = stmtScope.expr, stmtScope.makePhysicalProps() - rtyp := f.ResolvedType() + rTyp := f.ResolvedType() // Add a LIMIT 1 to the last statement if the UDF is not // set-returning. This is valid because any other rows after the @@ -515,76 +512,170 @@ func (b *Builder) finishBuildLastStmt( physProps.Ordering = props.OrderingChoice{} } - // Only a single column can be returned from a UDF, unless it is used as a - // data source. Data sources may output multiple columns, and if the - // statement body produces a tuple it needs to be expanded into columns. - // When not used as a data source, combine statements producing multiple - // columns into a tuple. If the last statement is already returning a - // tuple and the function has a record return type, then we do not need to - // wrap the output in another tuple. + // Depending on the context in which the UDF was called, it may be necessary + // to either combine multiple result columns into a tuple, or to expand a + // tuple result column into multiple columns. cols := physProps.Presentation - isSingleTupleResult := len(stmtScope.cols) == 1 && - stmtScope.cols[0].typ.Family() == types.TupleFamily - if insideDataSource && rtyp.Family() == types.TupleFamily { - // When the UDF is used as a data source and expects to output a tuple - // type, its output needs to be a row of columns instead of the usual - // tuple. If the last statement output a tuple, we need to expand the - // tuple into individual columns. - isMultiColDataSource = true - if isSingleTupleResult { - stmtScope = bodyScope.push() - elems := make([]scopeColumn, len(rtyp.TupleContents())) - for i := range rtyp.TupleContents() { - e := b.factory.ConstructColumnAccess(b.factory.ConstructVariable(cols[0].ID), memo.TupleOrdinal(i)) - col := b.synthesizeColumn(stmtScope, scopeColName(""), rtyp.TupleContents()[i], nil, e) - elems[i] = *col - } - expr = b.constructProject(expr, elems) - physProps = stmtScope.makePhysicalProps() + scopeCols := stmtScope.cols + isSingleTupleResult := len(scopeCols) == 1 && scopeCols[0].typ.Family() == types.TupleFamily + if insideDataSource { + // The UDF is a data source. If it returns a composite type and the last + // statement returns a single tuple column, the elements of the column + // should be expanded into individual columns. + if rTyp.Family() == types.TupleFamily && isSingleTupleResult { + expr, physProps = b.expandRoutineTupleIntoCols(cols[0].ID, bodyScope.push(), expr) } - } else if len(cols) > 1 || (types.IsRecordType(rtyp) && !isSingleTupleResult) { - // Only a single column can be returned from a UDF, unless it is used as a - // data source (see comment above). If there are multiple columns, combine - // them into a tuple. If the last statement is already returning a tuple - // and the function has a record return type, then do not wrap the - // output in another tuple. - elems := make(memo.ScalarListExpr, len(cols)) - for i := range cols { - elems[i] = b.factory.ConstructVariable(cols[i].ID) + } else { + // Only a single column can be returned from a routine, unless it is a UDF + // used as a data source (see comment above). There are three cases in which + // we must wrap the column(s) from the last statement into a single tuple: + // 1. The last statement has multiple result columns. + // 2. The routine returns RECORD, and the (single) result column cannot + // be coerced to the return type. Note that a procedure with OUT-params + // always wraps the OUT-param types in a record. + if len(cols) > 1 || (rTyp.Family() == types.TupleFamily && !scopeCols[0].typ.Equivalent(rTyp) && + !cast.ValidCast(scopeCols[0].typ, rTyp, cast.ContextAssignment)) { + expr, physProps = b.combineRoutineColsIntoTuple(cols, bodyScope.push(), expr) } - tup := b.factory.ConstructTuple(elems, rtyp) - stmtScope = bodyScope.push() - col := b.synthesizeColumn(stmtScope, scopeColName(""), rtyp, nil /* expr */, tup) - expr = b.constructProject(expr, []scopeColumn{*col}) - physProps = stmtScope.makePhysicalProps() } - // We must preserve the presentation of columns as physical - // properties to prevent the optimizer from pruning the output - // column. If necessary, we add an assignment cast to the result - // column so that its type matches the function return type. Record return - // types do not need an assignment cast, since at this point the return - // column is already a tuple. + // We must preserve the presentation of columns as physical properties to + // prevent the optimizer from pruning the output column(s). If necessary, we + // add an assignment cast to the result column(s) so that its type matches the + // function return type. cols = physProps.Presentation - if len(cols) > 0 { - returnCol := physProps.Presentation[0].ID - returnColMeta := b.factory.Metadata().ColumnMeta(returnCol) - if !types.IsRecordType(rtyp) && !isMultiColDataSource && !returnColMeta.Type.Identical(rtyp) { - if !cast.ValidCast(returnColMeta.Type, rtyp, cast.ContextAssignment) { - panic(sqlerrors.NewInvalidAssignmentCastError( - returnColMeta.Type, rtyp, returnColMeta.Alias)) + return b.maybeAddRoutineAssignmentCasts(cols, bodyScope, rTyp, expr, physProps, insideDataSource) +} + +// finalizeRoutineReturnType updates the routine's return type, taking into +// account the result columns of the last statement, as well as the column +// definition list if one was specified. +func (b *Builder) finalizeRoutineReturnType( + f *tree.FuncExpr, stmtScope, inScope *scope, insideDataSource bool, +) { + // If the function was defined using the wildcard RETURNS RECORD option with + // no OUT-parameters, its actual return type is inferred either from a + // column-definition list or from the types of the columns in the last + // statement. This is necessary because wildcard types are only valid during + // type-checking; the execution engine cannot handle them. + rTyp := f.ResolvedType() + if rTyp.Identical(types.AnyTuple) { + if len(stmtScope.cols) == 1 && stmtScope.cols[0].typ.Family() == types.TupleFamily { + // When the final statement returns a single tuple column, the column's + // type becomes the routine's return type. + rTyp = stmtScope.cols[0].typ + } else { + // Get the types from the columns of the last statement. + tc := make([]*types.T, len(stmtScope.cols)) + tl := make([]string, len(stmtScope.cols)) + for i, col := range stmtScope.cols { + tc[i] = col.typ + tl[i] = col.name.MetadataName() + } + rTyp = types.MakeLabeledTuple(tc, tl) + } + } + if insideDataSource { + // If the routine is used as a data source, there must be a column + // definition list, which must be compatible with the last statement's + // columns. + b.validateGeneratorFunctionReturnType(f.ResolvedOverload(), rTyp, inScope) + if f.ResolvedOverload().ReturnsRecordType { + // The validation happens for every routine used as a data source, but we + // only update the type using the column definition list for + // RECORD-returning routines. + rTyp = b.getColumnDefinitionListTypes(inScope) + } + } + f.SetTypeAnnotation(rTyp) +} + +// combineRoutineColsIntoTuple is a helper to combine individual result columns +// into a single tuple column. +func (b *Builder) combineRoutineColsIntoTuple( + cols physical.Presentation, stmtScope *scope, inputExpr memo.RelExpr, +) (memo.RelExpr, *physical.Required) { + elems := make(memo.ScalarListExpr, len(cols)) + typContents := make([]*types.T, len(cols)) + for i := range cols { + elems[i] = b.factory.ConstructVariable(cols[i].ID) + typContents[i] = b.factory.Metadata().ColumnMeta(cols[i].ID).Type + } + colTyp := types.MakeTuple(typContents) + tup := b.factory.ConstructTuple(elems, colTyp) + col := b.synthesizeColumn(stmtScope, scopeColName(""), colTyp, nil /* expr */, tup) + return b.constructProject(inputExpr, []scopeColumn{*col}), stmtScope.makePhysicalProps() +} + +// expandRoutineTupleIntoCols is a helper to expand the elements of a single +// tuple result column into individual result columns. +func (b *Builder) expandRoutineTupleIntoCols( + tupleColID opt.ColumnID, stmtScope *scope, inputExpr memo.RelExpr, +) (memo.RelExpr, *physical.Required) { + colTyp := b.factory.Metadata().ColumnMeta(tupleColID).Type + elems := make([]scopeColumn, len(colTyp.TupleContents())) + for i := range colTyp.TupleContents() { + varExpr := b.factory.ConstructVariable(tupleColID) + e := b.factory.ConstructColumnAccess(varExpr, memo.TupleOrdinal(i)) + col := b.synthesizeColumn(stmtScope, scopeColName(""), colTyp.TupleContents()[i], nil, e) + elems[i] = *col + } + return b.constructProject(inputExpr, elems), stmtScope.makePhysicalProps() +} + +// maybeAddRoutineAssignmentCasts checks whether the result columns of the last +// statement in a routine match up with the return type. If not, it attempts to +// assignment-cast the columns to the correct type. +func (b *Builder) maybeAddRoutineAssignmentCasts( + cols physical.Presentation, + bodyScope *scope, + rTyp *types.T, + expr memo.RelExpr, + physProps *physical.Required, + insideDataSource bool, +) (memo.RelExpr, *physical.Required) { + if rTyp.Family() == types.VoidFamily { + // Void routines don't return a result, so a cast is not necessary. + return expr, physProps + } + var desiredTypes []*types.T + if insideDataSource && rTyp.Family() == types.TupleFamily { + // The result column(s) should match the elements of the composite return + // type. + desiredTypes = rTyp.TupleContents() + } else { + // There should be a single result column that directly matches the return + // type. + desiredTypes = []*types.T{rTyp} + } + if len(desiredTypes) != len(cols) { + panic(errors.AssertionFailedf("expected types and cols to be the same length")) + } + needCast := false + md := b.factory.Metadata() + for i, col := range cols { + colTyp, expectedTyp := md.ColumnMeta(col.ID).Type, desiredTypes[i] + if !colTyp.Identical(expectedTyp) { + needCast = true + break + } + } + if !needCast { + return expr, physProps + } + stmtScope := bodyScope.push() + for i, col := range cols { + colTyp, expectedTyp := md.ColumnMeta(col.ID).Type, desiredTypes[i] + scalar := b.factory.ConstructVariable(cols[i].ID) + if !colTyp.Identical(expectedTyp) { + if !cast.ValidCast(colTyp, expectedTyp, cast.ContextAssignment) { + panic(errors.AssertionFailedf("invalid cast should have been caught earlier")) } - cast := b.factory.ConstructAssignmentCast( - b.factory.ConstructVariable(physProps.Presentation[0].ID), - rtyp, - ) - stmtScope = bodyScope.push() - col := b.synthesizeColumn(stmtScope, scopeColName(""), rtyp, nil /* expr */, cast) - expr = b.constructProject(expr, []scopeColumn{*col}) - physProps = stmtScope.makePhysicalProps() + scalar = b.factory.ConstructAssignmentCast(scalar, expectedTyp) } + b.synthesizeColumn(stmtScope, scopeColName(""), expectedTyp, nil /* expr */, scalar) } - return expr, physProps, isMultiColDataSource + return b.constructProject(expr, stmtScope.cols), stmtScope.makePhysicalProps() } func (b *Builder) withinSQLRoutine(fn func()) { diff --git a/pkg/sql/opt/optbuilder/scalar.go b/pkg/sql/opt/optbuilder/scalar.go index 646299e4d44e..85441b076aa7 100644 --- a/pkg/sql/opt/optbuilder/scalar.go +++ b/pkg/sql/opt/optbuilder/scalar.go @@ -574,7 +574,14 @@ func (b *Builder) buildFunction( }) if overload.Class == tree.GeneratorClass { - return b.finishBuildGeneratorFunction(f, overload, out, inScope, outScope, outCol) + if overload.ReturnsRecordType { + if colDefListTypes := b.getColumnDefinitionListTypes(inScope); colDefListTypes != nil { + // Use the types from the column definition list to determine the + // function return type. + f.SetTypeAnnotation(colDefListTypes) + } + } + return b.finishBuildGeneratorFunction(f, out, inScope, outScope, outCol) } // Add a dependency on sequences that are used as a string argument. @@ -609,6 +616,27 @@ func (b *Builder) buildFunction( return b.finishBuildScalar(f, out, inScope, outScope, outCol) } +// getColumnDefinitionListTypes returns a composite type representing the column +// definition list for the current scope, if any. If one doesn't exist, +// getColumnDefinitionListTypes returns nil. +func (b *Builder) getColumnDefinitionListTypes(inScope *scope) *types.T { + alias := inScope.alias + if alias == nil || len(alias.Cols) == 0 || alias.Cols[0].Type == nil { + return nil + } + contents := make([]*types.T, len(alias.Cols)) + labels := make([]string, len(alias.Cols)) + for i, c := range alias.Cols { + defTyp, err := tree.ResolveType(b.ctx, c.Type, b.semaCtx.TypeResolver) + if err != nil { + panic(err) + } + contents[i] = defTyp + labels[i] = string(c.Name) + } + return types.MakeLabeledTuple(contents, labels) +} + // buildRangeCond builds a RANGE clause as a simpler expression. Examples: // x BETWEEN a AND b -> x >= a AND x <= b // x NOT BETWEEN a AND b -> NOT (x >= a AND x <= b) diff --git a/pkg/sql/opt/optbuilder/srfs.go b/pkg/sql/opt/optbuilder/srfs.go index 5ef4dbdbe801..b242264c57f3 100644 --- a/pkg/sql/opt/optbuilder/srfs.go +++ b/pkg/sql/opt/optbuilder/srfs.go @@ -17,10 +17,12 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/opt/memo" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" + "github.com/cockroachdb/cockroach/pkg/sql/sem/cast" "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented" "github.com/cockroachdb/errors" + "github.com/cockroachdb/redact" ) // srf represents an srf expression in an expression tree @@ -189,68 +191,110 @@ func (b *Builder) buildZip(exprs tree.Exprs, inScope *scope) (outScope *scope) { // (SRF) such as generate_series() or unnest(). It synthesizes new columns in // outScope for each of the SRF's output columns. func (b *Builder) finishBuildGeneratorFunction( - f *tree.FuncExpr, - def *tree.Overload, - fn opt.ScalarExpr, - inScope, outScope *scope, - outCol *scopeColumn, + f *tree.FuncExpr, fn opt.ScalarExpr, inScope, outScope *scope, outCol *scopeColumn, ) (out opt.ScalarExpr) { - lastAlias := inScope.alias - if def.ReturnsRecordType { - if lastAlias == nil { - var numOutParams int - for _, param := range def.RoutineParams { - if param.IsOutParam() { - numOutParams++ - } - if numOutParams == 2 { - break - } - } - // If we have at least two OUT parameters, they specify an implicit - // alias for the RECORD return type. - if numOutParams < 2 { - panic(pgerror.New(pgcode.Syntax, "a column definition list is required for functions returning \"record\"")) - } + rTyp := f.ResolvedType() + b.validateGeneratorFunctionReturnType(f.ResolvedOverload(), rTyp, inScope) + + // Add scope columns. + if outCol != nil { + // Single-column return type. + b.populateSynthesizedColumn(outCol, fn) + } else { + // Multi-column return type. Note that we already reconciled the function's + // return type with the column definition list (if it exists). + for i := range rTyp.TupleContents() { + colName := scopeColName(tree.Name(rTyp.TupleLabels()[i])) + b.synthesizeColumn(outScope, colName, rTyp.TupleContents()[i], nil, fn) } - } else if lastAlias != nil { - // Non-record type return with a table alias that includes types is not - // permitted. + } + return fn +} + +// validateGeneratorFunctionReturnType checks for various errors that result +// from the presence or absence of a column definition list and its +// compatibility with the actual function return type. This logic mirrors that +// in postgres. +func (b *Builder) validateGeneratorFunctionReturnType( + overload *tree.Overload, rTyp *types.T, inScope *scope, +) { + lastAlias := inScope.alias + hasColumnDefinitionList := false + if lastAlias != nil { for _, c := range lastAlias.Cols { if c.Type != nil { - panic(pgerror.Newf(pgcode.Syntax, "a column definition list is only allowed for functions returning \"record\"")) + hasColumnDefinitionList = true + break } } } - // Add scope columns. - if outCol != nil { - // Single-column return type. - b.populateSynthesizedColumn(outCol, fn) - } else if def.ReturnsRecordType && lastAlias != nil && len(lastAlias.Cols) > 0 { - // If we're building a generator function that returns a record type, like - // json_to_record, we need to know the alias that was assigned to the - // generator function - without that, we won't know the list of columns - // to output. - for _, c := range lastAlias.Cols { - if c.Type == nil { - panic(pgerror.Newf(pgcode.Syntax, "a column definition list is required for functions returning \"record\"")) + + // Validate the column definition list against the concrete return type of the + // function. + if hasColumnDefinitionList { + if !overload.ReturnsRecordType { + // Non RECORD-return type with a column definition list is not permitted. + for _, param := range overload.RoutineParams { + if param.IsOutParam() { + panic(pgerror.New(pgcode.Syntax, + "a column definition list is redundant for a function with OUT parameters", + )) + } } - typ, err := tree.ResolveType(b.ctx, c.Type, b.semaCtx.TypeResolver) - if err != nil { - panic(err) + if rTyp.Family() == types.TupleFamily { + panic(pgerror.New(pgcode.Syntax, + "a column definition list is redundant for a function returning a named composite type", + )) + } else { + panic(pgerror.Newf(pgcode.Syntax, + "a column definition list is only allowed for functions returning \"record\"", + )) } - b.synthesizeColumn(outScope, scopeColName(c.Name), typ, nil, fn) } - } else { - // Multi-column return type. Use the tuple labels in the SRF's return type - // as column aliases. - typ := f.ResolvedType() - for i := range typ.TupleContents() { - b.synthesizeColumn(outScope, scopeColName(tree.Name(typ.TupleLabels()[i])), typ.TupleContents()[i], nil, fn) + if len(rTyp.TupleContents()) != len(lastAlias.Cols) { + switch overload.Language { + case tree.RoutineLangSQL: + err := pgerror.New(pgcode.DatatypeMismatch, + "function return row and query-specified return row do not match", + ) + panic(errors.WithDetailf(err, + "Returned row contains %d attributes, but query expects %d.", + len(rTyp.TupleContents()), len(lastAlias.Cols), + )) + case tree.RoutineLangPLpgSQL: + err := pgerror.New(pgcode.DatatypeMismatch, + "returned record type does not match expected record type", + ) + panic(errors.WithDetailf(err, + "Number of returned columns (%d) does not match expected column count (%d).", + len(rTyp.TupleContents()), len(lastAlias.Cols), + )) + default: + panic(errors.AssertionFailedf( + "unexpected language: %s", redact.SafeString(overload.Language), + )) + } } + } else if overload.ReturnsRecordType { + panic(pgerror.New(pgcode.Syntax, + "a column definition list is required for functions returning \"record\"", + )) } - return fn + // Verify that the function return type can be assignment-casted to the + // column definition list type. + if hasColumnDefinitionList { + colDefListTypes := b.getColumnDefinitionListTypes(inScope) + for i := range colDefListTypes.TupleContents() { + colTyp, defTyp := rTyp.TupleContents()[i], colDefListTypes.TupleContents()[i] + if !colTyp.Identical(defTyp) && !cast.ValidCast(colTyp, defTyp, cast.ContextAssignment) { + panic(errors.WithDetailf(pgerror.New(pgcode.InvalidFunctionDefinition, + "return type mismatch in function declared to return record", + ), "Final statement returns %v instead of %v at column %d.", + colTyp.SQLStringForError(), defTyp.SQLStringForError(), i+1)) + } + } + } } // buildProjectSet builds a ProjectSet, which is a lateral cross join diff --git a/pkg/sql/opt/optbuilder/testdata/procedure b/pkg/sql/opt/optbuilder/testdata/procedure index d5a4e1ca0e70..5bac96a1131e 100644 --- a/pkg/sql/opt/optbuilder/testdata/procedure +++ b/pkg/sql/opt/optbuilder/testdata/procedure @@ -38,18 +38,13 @@ call │ ├── const: 1 │ ├── const: 2 │ └── const: 3 - └── project - ├── columns: column10:10 - ├── limit + └── limit + ├── columns: column1:9 + ├── values │ ├── columns: column1:9 - │ ├── values - │ │ ├── columns: column1:9 - │ │ └── tuple - │ │ └── null - │ └── const: 1 - └── projections - └── assignment-cast: VOID [as=column10:10] - └── variable: column1:9 + │ └── tuple + │ └── null + └── const: 1 exec-ddl CREATE OR REPLACE PROCEDURE p() LANGUAGE SQL AS $$ @@ -92,18 +87,13 @@ call │ ├── const: 7 │ ├── const: 8 │ └── const: 9 - └── project - ├── columns: column18:18 - ├── limit + └── limit + ├── columns: column1:17 + ├── values │ ├── columns: column1:17 - │ ├── values - │ │ ├── columns: column1:17 - │ │ └── tuple - │ │ └── null - │ └── const: 1 - └── projections - └── assignment-cast: VOID [as=column18:18] - └── variable: column1:17 + │ └── tuple + │ └── null + └── const: 1 # Procedure with arguments. exec-ddl @@ -142,15 +132,10 @@ call │ └── projections │ └── assignment-cast: INT8 [as=c_cast:12] │ └── variable: column3:11 - └── project - ├── columns: column14:14 - ├── limit + └── limit + ├── columns: column1:13 + ├── values │ ├── columns: column1:13 - │ ├── values - │ │ ├── columns: column1:13 - │ │ └── tuple - │ │ └── null - │ └── const: 1 - └── projections - └── assignment-cast: VOID [as=column14:14] - └── variable: column1:13 + │ └── tuple + │ └── null + └── const: 1 diff --git a/pkg/sql/opt/optbuilder/testdata/udf b/pkg/sql/opt/optbuilder/testdata/udf index a1e274ebb171..57a5e606fea6 100644 --- a/pkg/sql/opt/optbuilder/testdata/udf +++ b/pkg/sql/opt/optbuilder/testdata/udf @@ -855,36 +855,41 @@ build format=show-scalars SELECT get_abc(3) ---- project - ├── columns: get_abc:8 + ├── columns: get_abc:9 ├── values │ └── tuple └── projections - └── udf: get_abc [as=get_abc:8] + └── udf: get_abc [as=get_abc:9] ├── args │ └── const: 3 ├── params: i:1 └── body └── project - ├── columns: column7:7 - ├── limit - │ ├── columns: a:2!null b:3 c:4!null - │ ├── internal-ordering: -3 - │ ├── project + ├── columns: column8:8 + ├── project + │ ├── columns: column7:7 + │ ├── limit │ │ ├── columns: a:2!null b:3 c:4!null - │ │ └── select - │ │ ├── columns: a:2!null b:3 c:4!null crdb_internal_mvcc_timestamp:5 tableoid:6 - │ │ ├── scan abc - │ │ │ └── columns: a:2!null b:3 c:4 crdb_internal_mvcc_timestamp:5 tableoid:6 - │ │ └── filters - │ │ └── gt - │ │ ├── variable: c:4 - │ │ └── variable: i:1 - │ └── const: 1 + │ │ ├── internal-ordering: -3 + │ │ ├── project + │ │ │ ├── columns: a:2!null b:3 c:4!null + │ │ │ └── select + │ │ │ ├── columns: a:2!null b:3 c:4!null crdb_internal_mvcc_timestamp:5 tableoid:6 + │ │ │ ├── scan abc + │ │ │ │ └── columns: a:2!null b:3 c:4 crdb_internal_mvcc_timestamp:5 tableoid:6 + │ │ │ └── filters + │ │ │ └── gt + │ │ │ ├── variable: c:4 + │ │ │ └── variable: i:1 + │ │ └── const: 1 + │ └── projections + │ └── tuple [as=column7:7] + │ ├── variable: a:2 + │ ├── variable: b:3 + │ └── variable: c:4 └── projections - └── tuple [as=column7:7] - ├── variable: a:2 - ├── variable: b:3 - └── variable: c:4 + └── assignment-cast: RECORD [as=column8:8] + └── variable: column7:7 exec-ddl CREATE FUNCTION get_abc_star(i INT) RETURNS abc LANGUAGE SQL AS $$ @@ -896,36 +901,41 @@ build format=show-scalars SELECT get_abc_star(3) ---- project - ├── columns: get_abc_star:8 + ├── columns: get_abc_star:9 ├── values │ └── tuple └── projections - └── udf: get_abc_star [as=get_abc_star:8] + └── udf: get_abc_star [as=get_abc_star:9] ├── args │ └── const: 3 ├── params: i:1 └── body └── project - ├── columns: column7:7 - ├── limit - │ ├── columns: a:2!null b:3 c:4!null - │ ├── internal-ordering: -3 - │ ├── project + ├── columns: column8:8 + ├── project + │ ├── columns: column7:7 + │ ├── limit │ │ ├── columns: a:2!null b:3 c:4!null - │ │ └── select - │ │ ├── columns: a:2!null b:3 c:4!null crdb_internal_mvcc_timestamp:5 tableoid:6 - │ │ ├── scan abc - │ │ │ └── columns: a:2!null b:3 c:4 crdb_internal_mvcc_timestamp:5 tableoid:6 - │ │ └── filters - │ │ └── gt - │ │ ├── variable: c:4 - │ │ └── variable: i:1 - │ └── const: 1 + │ │ ├── internal-ordering: -3 + │ │ ├── project + │ │ │ ├── columns: a:2!null b:3 c:4!null + │ │ │ └── select + │ │ │ ├── columns: a:2!null b:3 c:4!null crdb_internal_mvcc_timestamp:5 tableoid:6 + │ │ │ ├── scan abc + │ │ │ │ └── columns: a:2!null b:3 c:4 crdb_internal_mvcc_timestamp:5 tableoid:6 + │ │ │ └── filters + │ │ │ └── gt + │ │ │ ├── variable: c:4 + │ │ │ └── variable: i:1 + │ │ └── const: 1 + │ └── projections + │ └── tuple [as=column7:7] + │ ├── variable: a:2 + │ ├── variable: b:3 + │ └── variable: c:4 └── projections - └── tuple [as=column7:7] - ├── variable: a:2 - ├── variable: b:3 - └── variable: c:4 + └── assignment-cast: RECORD [as=column8:8] + └── variable: column7:7 exec-ddl CREATE FUNCTION abc_b(i abc) RETURNS INT LANGUAGE SQL AS $$ @@ -1043,11 +1053,11 @@ build format=show-scalars SELECT retvoid(1) ---- project - ├── columns: retvoid:5 + ├── columns: retvoid:4 ├── values │ └── tuple └── projections - └── udf: retvoid [as=retvoid:5] + └── udf: retvoid [as=retvoid:4] ├── args │ └── const: 1 ├── params: i:1 @@ -1058,18 +1068,13 @@ project │ │ └── tuple │ └── projections │ └── variable: i:1 [as=i:2] - └── project - ├── columns: column4:4 - ├── limit + └── limit + ├── columns: column1:3 + ├── values │ ├── columns: column1:3 - │ ├── values - │ │ ├── columns: column1:3 - │ │ └── tuple - │ │ └── null - │ └── const: 1 - └── projections - └── assignment-cast: VOID [as=column4:4] - └── variable: column1:3 + │ └── tuple + │ └── null + └── const: 1 # -------------------------------------------------- @@ -1243,11 +1248,11 @@ build format=show-scalars SELECT * FROM (SELECT strict_fn_record(1, 'foo', false)) as bar; ---- project - ├── columns: strict_fn_record:8 + ├── columns: strict_fn_record:9 ├── values │ └── tuple └── projections - └── udf: strict_fn_record [as=strict_fn_record:8] + └── udf: strict_fn_record [as=strict_fn_record:9] ├── strict ├── args │ ├── const: 1 @@ -1256,23 +1261,28 @@ project ├── params: i:1 t:2 b:3 └── body └── project - ├── columns: column7:7 - ├── limit - │ ├── columns: i:4 t:5 b:6 - │ ├── project + ├── columns: column8:8 + ├── project + │ ├── columns: column7:7 + │ ├── limit │ │ ├── columns: i:4 t:5 b:6 - │ │ ├── values - │ │ │ └── tuple - │ │ └── projections - │ │ ├── variable: i:1 [as=i:4] - │ │ ├── variable: t:2 [as=t:5] - │ │ └── variable: b:3 [as=b:6] - │ └── const: 1 + │ │ ├── project + │ │ │ ├── columns: i:4 t:5 b:6 + │ │ │ ├── values + │ │ │ │ └── tuple + │ │ │ └── projections + │ │ │ ├── variable: i:1 [as=i:4] + │ │ │ ├── variable: t:2 [as=t:5] + │ │ │ └── variable: b:3 [as=b:6] + │ │ └── const: 1 + │ └── projections + │ └── tuple [as=column7:7] + │ ├── variable: i:4 + │ ├── variable: t:5 + │ └── variable: b:6 └── projections - └── tuple [as=column7:7] - ├── variable: i:4 - ├── variable: t:5 - └── variable: b:6 + └── assignment-cast: RECORD [as=column8:8] + └── variable: column7:7 # -------------------------------------------------- @@ -1315,44 +1325,49 @@ CREATE TABLE tstar2 ( ---- exec-ddl -CREATE FUNCTION fn_star2() RETURNS INT LANGUAGE SQL AS 'SELECT * FROM tstar, tstar2 WHERE tstar.a = tstar2.b' +CREATE FUNCTION fn_star2() RETURNS RECORD LANGUAGE SQL AS 'SELECT * FROM tstar, tstar2 WHERE tstar.a = tstar2.b' ---- build format=show-scalars SELECT fn_star2() ---- project - ├── columns: fn_star2:11 + ├── columns: fn_star2:12 ├── values │ └── tuple └── projections - └── udf: fn_star2 [as=fn_star2:11] + └── udf: fn_star2 [as=fn_star2:12] └── body └── project - ├── columns: column10:10 - ├── limit - │ ├── columns: tstar.a:1!null tstar2.a:5 b:6!null - │ ├── project + ├── columns: column11:11 + ├── project + │ ├── columns: column10:10 + │ ├── limit │ │ ├── columns: tstar.a:1!null tstar2.a:5 b:6!null - │ │ └── select - │ │ ├── columns: tstar.a:1!null tstar.rowid:2!null tstar.crdb_internal_mvcc_timestamp:3 tstar.tableoid:4 tstar2.a:5 b:6!null tstar2.rowid:7!null tstar2.crdb_internal_mvcc_timestamp:8 tstar2.tableoid:9 - │ │ ├── inner-join (cross) - │ │ │ ├── columns: tstar.a:1 tstar.rowid:2!null tstar.crdb_internal_mvcc_timestamp:3 tstar.tableoid:4 tstar2.a:5 b:6 tstar2.rowid:7!null tstar2.crdb_internal_mvcc_timestamp:8 tstar2.tableoid:9 - │ │ │ ├── scan tstar - │ │ │ │ └── columns: tstar.a:1 tstar.rowid:2!null tstar.crdb_internal_mvcc_timestamp:3 tstar.tableoid:4 - │ │ │ ├── scan tstar2 - │ │ │ │ └── columns: tstar2.a:5 b:6 tstar2.rowid:7!null tstar2.crdb_internal_mvcc_timestamp:8 tstar2.tableoid:9 - │ │ │ └── filters (true) - │ │ └── filters - │ │ └── eq - │ │ ├── variable: tstar.a:1 - │ │ └── variable: b:6 - │ └── const: 1 + │ │ ├── project + │ │ │ ├── columns: tstar.a:1!null tstar2.a:5 b:6!null + │ │ │ └── select + │ │ │ ├── columns: tstar.a:1!null tstar.rowid:2!null tstar.crdb_internal_mvcc_timestamp:3 tstar.tableoid:4 tstar2.a:5 b:6!null tstar2.rowid:7!null tstar2.crdb_internal_mvcc_timestamp:8 tstar2.tableoid:9 + │ │ │ ├── inner-join (cross) + │ │ │ │ ├── columns: tstar.a:1 tstar.rowid:2!null tstar.crdb_internal_mvcc_timestamp:3 tstar.tableoid:4 tstar2.a:5 b:6 tstar2.rowid:7!null tstar2.crdb_internal_mvcc_timestamp:8 tstar2.tableoid:9 + │ │ │ │ ├── scan tstar + │ │ │ │ │ └── columns: tstar.a:1 tstar.rowid:2!null tstar.crdb_internal_mvcc_timestamp:3 tstar.tableoid:4 + │ │ │ │ ├── scan tstar2 + │ │ │ │ │ └── columns: tstar2.a:5 b:6 tstar2.rowid:7!null tstar2.crdb_internal_mvcc_timestamp:8 tstar2.tableoid:9 + │ │ │ │ └── filters (true) + │ │ │ └── filters + │ │ │ └── eq + │ │ │ ├── variable: tstar.a:1 + │ │ │ └── variable: b:6 + │ │ └── const: 1 + │ └── projections + │ └── tuple [as=column10:10] + │ ├── variable: tstar.a:1 + │ ├── variable: tstar2.a:5 + │ └── variable: b:6 └── projections - └── tuple [as=column10:10] - ├── variable: tstar.a:1 - ├── variable: tstar2.a:5 - └── variable: b:6 + └── assignment-cast: RECORD [as=column11:11] + └── variable: column10:10 # -------------------------------------------------- @@ -1740,11 +1755,11 @@ build set=enable_multiple_modifications_of_table=true SELECT upd_ups(1, 2, 3) ---- project - ├── columns: upd_ups:29 + ├── columns: upd_ups:28 ├── values │ └── () └── projections - └── upd_ups(1, 2, 3) [as=upd_ups:29] + └── upd_ups(1, 2, 3) [as=upd_ups:28] exec-ddl CREATE FUNCTION ups2(a1 INT, b1 INT, c1 INT, a2 INT, b2 INT, c2 INT) RETURNS BOOL LANGUAGE SQL AS $$ @@ -1803,7 +1818,7 @@ opt format=show-scalars SELECT f_fk(100, 1), f_fk(101, 2); ---- values - ├── columns: f_fk:14 f_fk:28 + ├── columns: f_fk:15 f_fk:30 └── tuple ├── udf: f_fk │ ├── args @@ -1812,7 +1827,7 @@ values │ ├── params: k:1 r:2 │ └── body │ └── project - │ ├── columns: column13:13!null + │ ├── columns: column14:14!null │ ├── insert child │ │ ├── columns: c:3!null child.p:4!null │ │ ├── insert-mapping: @@ -1843,47 +1858,49 @@ values │ │ ├── variable: p:9 │ │ └── variable: parent.p:10 │ └── projections - │ └── tuple [as=column13:13] - │ ├── variable: c:3 - │ └── variable: child.p:4 + │ └── assignment-cast: RECORD [as=column14:14] + │ └── tuple + │ ├── variable: c:3 + │ └── variable: child.p:4 └── udf: f_fk ├── args │ ├── const: 101 │ └── const: 2 - ├── params: k:15 r:16 + ├── params: k:16 r:17 └── body └── project - ├── columns: column27:27!null + ├── columns: column29:29!null ├── insert child - │ ├── columns: c:17!null child.p:18!null + │ ├── columns: c:18!null child.p:19!null │ ├── insert-mapping: - │ │ ├── column1:21 => c:17 - │ │ └── column2:22 => child.p:18 + │ │ ├── column1:22 => c:18 + │ │ └── column2:23 => child.p:19 │ ├── return-mapping: - │ │ ├── column1:21 => c:17 - │ │ └── column2:22 => child.p:18 + │ │ ├── column1:22 => c:18 + │ │ └── column2:23 => child.p:19 │ ├── input binding: &2 │ ├── values - │ │ ├── columns: column1:21 column2:22 + │ │ ├── columns: column1:22 column2:23 │ │ └── tuple - │ │ ├── variable: k:15 - │ │ └── variable: r:16 + │ │ ├── variable: k:16 + │ │ └── variable: r:17 │ └── f-k-checks │ └── f-k-checks-item: child(p) -> parent(p) │ └── anti-join (hash) - │ ├── columns: p:23 + │ ├── columns: p:24 │ ├── with-scan &2 - │ │ ├── columns: p:23 + │ │ ├── columns: p:24 │ │ └── mapping: - │ │ └── column2:22 => p:23 + │ │ └── column2:23 => p:24 │ ├── scan parent - │ │ ├── columns: parent.p:24!null + │ │ ├── columns: parent.p:25!null │ │ └── flags: disabled not visible index feature │ └── filters │ └── eq - │ ├── variable: p:23 - │ └── variable: parent.p:24 + │ ├── variable: p:24 + │ └── variable: parent.p:25 └── projections - └── tuple [as=column27:27] - ├── variable: c:17 - └── variable: child.p:18 + └── assignment-cast: RECORD [as=column29:29] + └── tuple + ├── variable: c:18 + └── variable: child.p:19 diff --git a/pkg/sql/opt/testutils/testcat/function.go b/pkg/sql/opt/testutils/testcat/function.go index bf90085e358c..c16cffab2aa2 100644 --- a/pkg/sql/opt/testutils/testcat/function.go +++ b/pkg/sql/opt/testutils/testcat/function.go @@ -181,7 +181,7 @@ func (tc *Catalog) CreateRoutine(c *tree.CreateRoutine) { OutParamTypes: outParams, DefaultExprs: defaultExprs, } - overload.ReturnsRecordType = types.IsRecordType(retType) + overload.ReturnsRecordType = retType.Identical(types.AnyTuple) if c.ReturnType != nil && c.ReturnType.SetOf { overload.Class = tree.GeneratorClass } diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/create_function.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/create_function.go index b334519a067e..cc597ff1cfcb 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/create_function.go +++ b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/create_function.go @@ -25,6 +25,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sqlerrors" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/errors" + "github.com/lib/pq/oid" ) func CreateFunction(b BuildCtx, n *tree.CreateRoutine) { @@ -66,7 +67,7 @@ func CreateFunction(b BuildCtx, n *tree.CreateRoutine) { if n.IsProcedure { if n.ReturnType != nil { returnType := b.ResolveTypeRef(n.ReturnType.Type) - if returnType.Type.Family() != types.VoidFamily && !types.IsRecordType(returnType.Type) { + if returnType.Type.Family() != types.VoidFamily && returnType.Type.Oid() != oid.T_record { panic(errors.AssertionFailedf( "CreateRoutine.ReturnType is expected to be empty, VOID, or RECORD for procedures", )) @@ -79,7 +80,7 @@ func CreateFunction(b BuildCtx, n *tree.CreateRoutine) { } } else if n.ReturnType != nil { typ = n.ReturnType.Type - if returnType := b.ResolveTypeRef(typ); types.IsRecordType(returnType.Type) { + if returnType := b.ResolveTypeRef(typ); returnType.Type.Oid() == oid.T_record { // If the function returns a RECORD type, then we need to check // whether its OUT parameters specify labels for the return type. outParamTypes, outParamNames := getOutputParameters(b, n.Params) diff --git a/pkg/sql/types/types.go b/pkg/sql/types/types.go index d369072759e0..f196842dcf50 100644 --- a/pkg/sql/types/types.go +++ b/pkg/sql/types/types.go @@ -2836,13 +2836,6 @@ func IsWildcardTupleType(t *T) bool { return len(t.TupleContents()) == 1 && t.TupleContents()[0].Family() == AnyFamily } -// IsRecordType returns true if this is a RECORD type. This should only be used -// when processing UDFs. A record differs from AnyTuple in that the tuple -// contents may contain types other than Any. -func IsRecordType(typ *T) bool { - return typ.Family() == TupleFamily && typ.Oid() == oid.T_record -} - // collatedStringTypeSQL returns the string representation of a COLLATEDSTRING // or []COLLATEDSTRING type. This is tricky in the case of an array of collated // string, since brackets must precede the COLLATE identifier: