Skip to content

Commit

Permalink
sql: handle implicit record return types better
Browse files Browse the repository at this point in the history
This PR validates the UDF return type at build time if it is a
user-defined type. If the return type is no longer compatible with what
the UDF body returns, we return an error instead. This is more in line
with postgres behavior.

Fixes: 95558
Epic: CRDB-19496
Release note (sql change): UDFs with implicit record return types will
return an error when called if the return type has been altered and is
no longer compatible with the body of the UDF.
  • Loading branch information
rharding6373 committed Feb 13, 2023
1 parent 0c2cc47 commit 86386f9
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 6 deletions.
47 changes: 47 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/udf
Expand Up @@ -3044,3 +3044,50 @@ query I
SELECT count(descriptor) FROM system.descriptor WHERE id = $dropped_fn_id;
----
0

subtest udt_alter

statement ok
CREATE TABLE t_alter (
a INT PRIMARY KEY,
b INT
)

statement ok
CREATE FUNCTION f_rtbl() RETURNS t_alter LANGUAGE SQL AS 'SELECT 1, 2;'

query T
SELECT f_rtbl();
----
(1,2)

statement ok
ALTER TABLE t_alter DROP COLUMN b;

statement error pq: return type mismatch in function declared to return t_alter
SELECT f_rtbl();

statement ok
ALTER TABLE t_alter ADD COLUMN b INT;

query T
SELECT f_rtbl();
----
(1,2)

statement ok
SET enable_experimental_alter_column_type_general=true

statement ok
ALTER TABLE t_alter ALTER b TYPE FLOAT;

statement error pq: return type mismatch in function declared to return t_alter
SELECT f_rtbl();

statement ok
ALTER TABLE t_alter ALTER b TYPE INT;

query T
SELECT f_rtbl();
----
(1,2)
16 changes: 10 additions & 6 deletions pkg/sql/logictest/testdata/logic_test/udf_star
Expand Up @@ -173,16 +173,20 @@ SELECT f_allcolsel_alias()
----
(1,2)

# Return an error after adding a column when the table is used as the return
# type. Note that this behavior is ok for late binding.
statement ok
ALTER TABLE t_twocol ADD COLUMN c INT DEFAULT 5;

# TODO(#95558): With early binding, postgres returns an error after adding a
# column when the table is used as the return type. Note that this behavior is
# ok for late binding.
query T
statement error pq: return type mismatch in function declared to return t_twocol
SELECT f_unqualified_twocol()

# Dropping the new column renders the function usable again.
statement ok
ALTER TABLE t_twocol DROP COLUMN c;

statement ok
SELECT f_unqualified_twocol()
----
(1,2)

# Altering a column type is not allowed in postgres or CRDB.
statement error pq: cannot alter type of column "b" because function "f_unqualified_twocol" depends on it
Expand Down
16 changes: 16 additions & 0 deletions pkg/sql/opt/optbuilder/scalar.go
Expand Up @@ -688,6 +688,22 @@ func (b *Builder) buildUDF(
// ConvertUDFToSubquery.
physProps.Ordering = props.OrderingChoice{}

// Validate that user defined return types match the original return types
// defined in the function.
rtyp := f.ResolvedType()
if rtyp.UserDefined() {
funcReturnType, err := tree.ResolveType(b.ctx,
&tree.OIDTypeReference{OID: rtyp.Oid()}, b.semaCtx.TypeResolver)
if err != nil {
panic(err)
}
if !funcReturnType.Equivalent(rtyp) {
panic(pgerror.Newf(
pgcode.InvalidFunctionDefinition,
"return type mismatch in function declared to return %s", rtyp.Name()))
}
}

// If there are multiple output columns, we must combine them into a
// tuple - only a single column can be returned from a UDF.
if cols := physProps.Presentation; len(cols) > 1 {
Expand Down

0 comments on commit 86386f9

Please sign in to comment.