Skip to content

Commit aab7109

Browse files
committed
sql: fix UDF with VOID return type
Functions with VOID returns types should be allowed to produce any number of columns of any type from their last statement. Prior to this commit, a calling UDF with a VOID return type would error if the last statement in the UDF did not produce a value of type VOID. This commit fixes this minor bug. Fixes cockroachdb#108297 Release note (bug fix): The last SQL statement in a user-defined function with a VOID return type can now produce any number of columns of any type. This bug was present since UDFs were introduced in version 22.2.
1 parent cdf6d15 commit aab7109

File tree

3 files changed

+106
-2
lines changed

3 files changed

+106
-2
lines changed

pkg/sql/logictest/testdata/logic_test/udf_regressions

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -580,3 +580,48 @@ statement error pgcode 22P02 invalid input value for enum e105259: "foo"
580580
SELECT f()
581581

582582
subtest end
583+
584+
585+
# Regression test for #108297. UDFs with VOID return types should succeed when
586+
# the last statement returns columns of any type.
587+
subtest regression_108297
588+
589+
statement ok
590+
CREATE OR REPLACE FUNCTION f108297() RETURNS VOID LANGUAGE SQL AS 'SELECT 1'
591+
592+
query T
593+
SELECT f108297()
594+
----
595+
NULL
596+
597+
statement ok
598+
CREATE OR REPLACE FUNCTION f108297() RETURNS VOID LANGUAGE SQL AS $$
599+
SELECT 1, 'foo', NULL
600+
$$
601+
602+
query T
603+
SELECT f108297()
604+
----
605+
NULL
606+
607+
statement ok
608+
CREATE SEQUENCE s108297
609+
610+
statement ok
611+
CREATE OR REPLACE FUNCTION f108297() RETURNS VOID LANGUAGE SQL AS $$
612+
SELECT nextval('s108297')
613+
$$
614+
615+
query T
616+
SELECT f108297()
617+
----
618+
NULL
619+
620+
# Invoking the UDF above should have increment s108297 to 1, so calling nextval
621+
# again should yield 2.
622+
query I
623+
SELECT nextval('s108297')
624+
----
625+
2
626+
627+
subtest end

pkg/sql/opt/optbuilder/scalar.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/cockroachdb/cockroach/pkg/sql/opt/props"
2525
"github.com/cockroachdb/cockroach/pkg/sql/opt/props/physical"
2626
"github.com/cockroachdb/cockroach/pkg/sql/parser"
27+
"github.com/cockroachdb/cockroach/pkg/sql/parser/statements"
2728
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
2829
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
2930
plpgsql "github.com/cockroachdb/cockroach/pkg/sql/plpgsql/parser"
@@ -728,6 +729,21 @@ func (b *Builder) buildUDF(
728729
if err != nil {
729730
panic(err)
730731
}
732+
// Add a VALUES (NULL) statement if the return type of the function is
733+
// VOID. We cant simply project NULL from the last statement because all
734+
// column would be pruned and the contents of last statement would not
735+
// be executed.
736+
// TODO(mgartner): This will add some planning overhead for every
737+
// invocation of the function. Is there a more efficient way to do this?
738+
if rtyp.Family() == types.VoidFamily {
739+
stmts = append(stmts, statements.Statement[tree.Statement]{
740+
AST: &tree.Select{
741+
Select: &tree.ValuesClause{
742+
Rows: []tree.Exprs{{tree.DNull}},
743+
},
744+
},
745+
})
746+
}
731747
body = make([]memo.RelExpr, len(stmts))
732748
bodyProps = make([]*physical.Required, len(stmts))
733749

@@ -750,6 +766,8 @@ func (b *Builder) buildUDF(
750766
if err != nil {
751767
panic(err)
752768
}
769+
// TODO(#108298): Figure out how to handle PLpgSQL functions with VOID
770+
// return types.
753771
var plBuilder plpgsqlBuilder
754772
plBuilder.init(b, colRefs, o.Types.(tree.ParamTypes), stmt.AST, rtyp)
755773
stmtScope := plBuilder.build(stmt.AST, bodyScope)

pkg/sql/opt/optbuilder/testdata/udf

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,6 +1031,47 @@ project
10311031
└── variable: s:2
10321032

10331033

1034+
# --------------------------------------------------
1035+
# UDFs with VOID return type.
1036+
# --------------------------------------------------
1037+
1038+
exec-ddl
1039+
CREATE FUNCTION retvoid(i INT) RETURNS VOID LANGUAGE SQL AS 'SELECT i'
1040+
----
1041+
1042+
build format=show-scalars
1043+
SELECT retvoid(1)
1044+
----
1045+
project
1046+
├── columns: retvoid:5
1047+
├── values
1048+
│ └── tuple
1049+
└── projections
1050+
└── udf: retvoid [as=retvoid:5]
1051+
├── args
1052+
│ └── const: 1
1053+
├── params: i:1
1054+
└── body
1055+
├── project
1056+
│ ├── columns: i:2
1057+
│ ├── values
1058+
│ │ └── tuple
1059+
│ └── projections
1060+
│ └── variable: i:1 [as=i:2]
1061+
└── project
1062+
├── columns: column4:4
1063+
├── limit
1064+
│ ├── columns: column1:3
1065+
│ ├── values
1066+
│ │ ├── columns: column1:3
1067+
│ │ └── tuple
1068+
│ │ └── null
1069+
│ └── const: 1
1070+
└── projections
1071+
└── assignment-cast: VOID [as=column4:4]
1072+
└── variable: column1:3
1073+
1074+
10341075
# --------------------------------------------------
10351076
# UDFs that are STRICT/RETURNS NULL ON NULL INPUT.
10361077
# --------------------------------------------------
@@ -1699,11 +1740,11 @@ build set=enable_multiple_modifications_of_table=true
16991740
SELECT upd_ups(1, 2, 3)
17001741
----
17011742
project
1702-
├── columns: upd_ups:27
1743+
├── columns: upd_ups:29
17031744
├── values
17041745
│ └── ()
17051746
└── projections
1706-
└── upd_ups(1, 2, 3) [as=upd_ups:27]
1747+
└── upd_ups(1, 2, 3) [as=upd_ups:29]
17071748

17081749
exec-ddl
17091750
CREATE FUNCTION ups2(a1 INT, b1 INT, c1 INT, a2 INT, b2 INT, c2 INT) RETURNS BOOL LANGUAGE SQL AS $$

0 commit comments

Comments
 (0)