diff --git a/pkg/sql/logictest/testdata/logic_test/udf_plpgsql b/pkg/sql/logictest/testdata/logic_test/udf_plpgsql index 187bc8dbbd22..624cd94a9ae1 100644 --- a/pkg/sql/logictest/testdata/logic_test/udf_plpgsql +++ b/pkg/sql/logictest/testdata/logic_test/udf_plpgsql @@ -1907,6 +1907,32 @@ SELECT f(); NOTICE: i = 104 NOTICE: i = 103 +# When the SQL statement returns zero rows, the target variables are all set +# to NULL. +statement ok +CREATE OR REPLACE FUNCTION f() RETURNS INT AS $$ + DECLARE + i INT := 0; + BEGIN + RAISE NOTICE 'i = %', i; + SELECT x INTO i FROM xy WHERE False; + RAISE NOTICE 'i = %', i; + i := 1; + RAISE NOTICE 'i = %', i; + INSERT INTO xy (SELECT 1, 2 FROM generate_series(1, 0)) RETURNING x INTO i; + RAISE NOTICE 'i = %', i; + RETURN 0; + END +$$ LANGUAGE PLpgSQL; + +query T noticetrace +SELECT f(); +---- +NOTICE: i = 0 +NOTICE: i = +NOTICE: i = 1 +NOTICE: i = + statement error pgcode 0A000 pq: unimplemented: assigning to a variable more than once in the same INTO statement is not supported CREATE OR REPLACE FUNCTION f() RETURNS INT AS $$ DECLARE @@ -2730,4 +2756,66 @@ SELECT f(); ---- 1 +# Regression test for #114826 - PLpgSQL routines should not prematurely exit +# when a SELECT INTO statement returns no rows. +subtest no_row_select_into + +statement ok +CREATE TABLE t114826 (a INT); + +statement ok +CREATE FUNCTION f114826() RETURNS INT AS $$ + DECLARE + found INT; + BEGIN + RAISE NOTICE 'HERE 1'; + SELECT a INTO found FROM t114826 WHERE a = 3; + RAISE NOTICE 'HERE 2'; + INSERT INTO t114826 (SELECT * FROM t114826) RETURNING a INTO found; + RAISE NOTICE 'HERE 3'; + RETURN 10; + END +$$ LANGUAGE PLpgSQL; + +query T noticetrace +SELECT f114826(); +---- +NOTICE: HERE 1 +NOTICE: HERE 2 +NOTICE: HERE 3 + +query I +SELECT f114826(); +---- +10 + +statement ok +INSERT INTO t114826 VALUES (1); + +query T noticetrace +SELECT f114826(); +---- +NOTICE: HERE 1 +NOTICE: HERE 2 +NOTICE: HERE 3 + +query I +SELECT * FROM t114826; +---- +1 +1 + +query I +SELECT f114826(); +---- +10 + +query I +SELECT * FROM t114826; +---- +1 +1 +1 +1 + subtest end diff --git a/pkg/sql/opt/norm/decorrelate_funcs.go b/pkg/sql/opt/norm/decorrelate_funcs.go index d8169ef5ff4c..be32df156bbe 100644 --- a/pkg/sql/opt/norm/decorrelate_funcs.go +++ b/pkg/sql/opt/norm/decorrelate_funcs.go @@ -742,10 +742,7 @@ func (c *CustomFuncs) ConstructBinary(op opt.Operator, left, right opt.ScalarExp // ConstructNoColsRow returns a Values operator having a single row with zero // columns. func (c *CustomFuncs) ConstructNoColsRow() memo.RelExpr { - return c.f.ConstructValues(memo.ScalarListWithEmptyTuple, &memo.ValuesPrivate{ - Cols: opt.ColList{}, - ID: c.f.Metadata().NextUniqueID(), - }) + return c.f.ConstructNoColsRow() } // referenceSingleColumn returns a Variable operator that refers to the one and diff --git a/pkg/sql/opt/norm/factory.go b/pkg/sql/opt/norm/factory.go index a92f82b998a4..e192ed2bba8d 100644 --- a/pkg/sql/opt/norm/factory.go +++ b/pkg/sql/opt/norm/factory.go @@ -464,6 +464,14 @@ func (f *Factory) ConstructZeroValues() memo.RelExpr { }) } +// ConstructNoColsRow constructs a Values operator with no columns and one row. +func (f *Factory) ConstructNoColsRow() memo.RelExpr { + return f.ConstructValues(memo.ScalarListWithEmptyTuple, &memo.ValuesPrivate{ + Cols: opt.ColList{}, + ID: f.Metadata().NextUniqueID(), + }) +} + // ConstructJoin constructs the join operator that corresponds to the given join // operator type. func (f *Factory) ConstructJoin( diff --git a/pkg/sql/opt/norm/factory_test.go b/pkg/sql/opt/norm/factory_test.go index bd1645958951..2a0b3af1cc0b 100644 --- a/pkg/sql/opt/norm/factory_test.go +++ b/pkg/sql/opt/norm/factory_test.go @@ -50,10 +50,7 @@ func TestSimplifyFilters(t *testing.T) { eq := f.ConstructEq(variable, constant) // Filters expression evaluates to False if any operand is False. - vals := f.ConstructValues(memo.ScalarListWithEmptyTuple, &memo.ValuesPrivate{ - Cols: opt.ColList{}, - ID: f.Metadata().NextUniqueID(), - }) + vals := f.ConstructNoColsRow() filters := memo.FiltersExpr{ f.ConstructFiltersItem(eq), f.ConstructFiltersItem(memo.FalseSingleton), diff --git a/pkg/sql/opt/optbuilder/insert.go b/pkg/sql/opt/optbuilder/insert.go index 0a9ce5e86018..b31af9279a6c 100644 --- a/pkg/sql/opt/optbuilder/insert.go +++ b/pkg/sql/opt/optbuilder/insert.go @@ -572,10 +572,7 @@ func (mb *mutationBuilder) buildInputForInsert(inScope *scope, inputRows *tree.S // Handle DEFAULT VALUES case by creating a single empty row as input. if inputRows == nil { mb.outScope = inScope.push() - mb.outScope.expr = mb.b.factory.ConstructValues(memo.ScalarListWithEmptyTuple, &memo.ValuesPrivate{ - Cols: opt.ColList{}, - ID: mb.md.NextUniqueID(), - }) + mb.outScope.expr = mb.b.factory.ConstructNoColsRow() mb.inputForInsertExpr = mb.outScope.expr return } diff --git a/pkg/sql/opt/optbuilder/plpgsql.go b/pkg/sql/opt/optbuilder/plpgsql.go index 172129742d2c..50120ba8145c 100644 --- a/pkg/sql/opt/optbuilder/plpgsql.go +++ b/pkg/sql/opt/optbuilder/plpgsql.go @@ -507,13 +507,23 @@ func (b *plpgsqlBuilder) buildPLpgSQLStatements(stmts []ast.Statement, s *scope) retCon := b.makeContinuation("_stmt_exec_ret") b.appendPlpgSQLStmts(&retCon, stmts[i+1:]) - // We only need the first row from the SQL statement. + // Ensure that the SQL statement returns at most one row. stmtScope.expr = b.ob.factory.ConstructLimit( stmtScope.expr, b.ob.factory.ConstructConst(tree.NewDInt(tree.DInt(1)), types.Int), stmtScope.makeOrderingChoice(), ) + // Ensure that the SQL statement returns at least one row. The RIGHT join + // ensures that when the SQL statement returns no rows, it is extended + // with a single row of NULL values. + stmtScope.expr = b.ob.factory.ConstructRightJoin( + stmtScope.expr, + b.ob.factory.ConstructNoColsRow(), + nil, /* on */ + memo.EmptyJoinPrivate, + ) + // Step 2: build the INTO statement into a continuation routine that calls // the previously built continuation. intoScope := b.buildInto(stmtScope, t.Target) @@ -1384,10 +1394,7 @@ func (b *plpgsqlBuilder) resolveVariableForAssign(name tree.Name) *types.T { func (b *plpgsqlBuilder) ensureScopeHasExpr(s *scope) { if s.expr == nil { - s.expr = b.ob.factory.ConstructValues(memo.ScalarListWithEmptyTuple, &memo.ValuesPrivate{ - Cols: opt.ColList{}, - ID: b.ob.factory.Metadata().NextUniqueID(), - }) + s.expr = b.ob.factory.ConstructNoColsRow() } } diff --git a/pkg/sql/opt/optbuilder/select.go b/pkg/sql/opt/optbuilder/select.go index dfed05de054b..6055c1d157de 100644 --- a/pkg/sql/opt/optbuilder/select.go +++ b/pkg/sql/opt/optbuilder/select.go @@ -1289,10 +1289,7 @@ func (b *Builder) buildFrom( outScope = b.buildFromTables(from.Tables, lockCtx, inScope) } else { outScope = inScope.push() - outScope.expr = b.factory.ConstructValues(memo.ScalarListWithEmptyTuple, &memo.ValuesPrivate{ - Cols: opt.ColList{}, - ID: b.factory.Metadata().NextUniqueID(), - }) + outScope.expr = b.factory.ConstructNoColsRow() } return outScope diff --git a/pkg/sql/opt/optbuilder/srfs.go b/pkg/sql/opt/optbuilder/srfs.go index 773880468926..5d429a14de4d 100644 --- a/pkg/sql/opt/optbuilder/srfs.go +++ b/pkg/sql/opt/optbuilder/srfs.go @@ -177,10 +177,7 @@ func (b *Builder) buildZip(exprs tree.Exprs, inScope *scope) (outScope *scope) { } // Construct the zip as a ProjectSet with empty input. - input := b.factory.ConstructValues(memo.ScalarListWithEmptyTuple, &memo.ValuesPrivate{ - Cols: opt.ColList{}, - ID: b.factory.Metadata().NextUniqueID(), - }) + input := b.factory.ConstructNoColsRow() outScope.expr = b.factory.ConstructProjectSet(input, zip) if len(outScope.cols) == 1 { outScope.singleSRFColumn = true diff --git a/pkg/sql/opt/optbuilder/testdata/procedure_plpgsql b/pkg/sql/opt/optbuilder/testdata/procedure_plpgsql index b2dcab302b4d..2b8be051ced4 100644 --- a/pkg/sql/opt/optbuilder/testdata/procedure_plpgsql +++ b/pkg/sql/opt/optbuilder/testdata/procedure_plpgsql @@ -55,23 +55,28 @@ call │ └── project │ ├── columns: "_stmt_exec_ret_2":58 │ ├── project - │ │ ├── columns: c:57!null - │ │ ├── limit - │ │ │ ├── columns: count_rows:14!null - │ │ │ ├── scalar-group-by + │ │ ├── columns: c:57 + │ │ ├── right-join (cross) + │ │ │ ├── columns: count_rows:14 + │ │ │ ├── limit │ │ │ │ ├── columns: count_rows:14!null - │ │ │ │ ├── project - │ │ │ │ │ └── select - │ │ │ │ │ ├── columns: k:9!null i:10 s:11 crdb_internal_mvcc_timestamp:12 tableoid:13 - │ │ │ │ │ ├── scan t - │ │ │ │ │ │ └── columns: k:9!null i:10 s:11 crdb_internal_mvcc_timestamp:12 tableoid:13 - │ │ │ │ │ └── filters - │ │ │ │ │ └── eq - │ │ │ │ │ ├── variable: k:9 - │ │ │ │ │ └── variable: arg_k:6 - │ │ │ │ └── aggregations - │ │ │ │ └── count-rows [as=count_rows:14] - │ │ │ └── const: 1 + │ │ │ │ ├── scalar-group-by + │ │ │ │ │ ├── columns: count_rows:14!null + │ │ │ │ │ ├── project + │ │ │ │ │ │ └── select + │ │ │ │ │ │ ├── columns: k:9!null i:10 s:11 crdb_internal_mvcc_timestamp:12 tableoid:13 + │ │ │ │ │ │ ├── scan t + │ │ │ │ │ │ │ └── columns: k:9!null i:10 s:11 crdb_internal_mvcc_timestamp:12 tableoid:13 + │ │ │ │ │ │ └── filters + │ │ │ │ │ │ └── eq + │ │ │ │ │ │ ├── variable: k:9 + │ │ │ │ │ │ └── variable: arg_k:6 + │ │ │ │ │ └── aggregations + │ │ │ │ │ └── count-rows [as=count_rows:14] + │ │ │ │ └── const: 1 + │ │ │ ├── values + │ │ │ │ └── tuple + │ │ │ └── filters (true) │ │ └── projections │ │ └── variable: count_rows:14 [as=c:57] │ └── projections diff --git a/pkg/sql/opt/optbuilder/testdata/udf_plpgsql b/pkg/sql/opt/optbuilder/testdata/udf_plpgsql index d028510fc1a7..4d355598d987 100644 --- a/pkg/sql/opt/optbuilder/testdata/udf_plpgsql +++ b/pkg/sql/opt/optbuilder/testdata/udf_plpgsql @@ -4433,14 +4433,19 @@ project │ ├── columns: "_stmt_exec_ret_2":15 │ ├── project │ │ ├── columns: i:13 j:14 - │ │ ├── limit + │ │ ├── right-join (cross) │ │ │ ├── columns: x:5 y:6 - │ │ │ ├── internal-ordering: -5 - │ │ │ ├── project + │ │ │ ├── limit │ │ │ │ ├── columns: x:5 y:6 - │ │ │ │ └── scan xy - │ │ │ │ └── columns: x:5 y:6 rowid:7!null crdb_internal_mvcc_timestamp:8 tableoid:9 - │ │ │ └── const: 1 + │ │ │ │ ├── internal-ordering: -5 + │ │ │ │ ├── project + │ │ │ │ │ ├── columns: x:5 y:6 + │ │ │ │ │ └── scan xy + │ │ │ │ │ └── columns: x:5 y:6 rowid:7!null crdb_internal_mvcc_timestamp:8 tableoid:9 + │ │ │ │ └── const: 1 + │ │ │ ├── values + │ │ │ │ └── tuple + │ │ │ └── filters (true) │ │ └── projections │ │ ├── variable: x:5 [as=i:13] │ │ └── variable: y:6 [as=j:14] @@ -4508,14 +4513,19 @@ project │ ├── columns: "_stmt_exec_ret_2":27 │ ├── project │ │ ├── columns: i:26 - │ │ ├── limit + │ │ ├── right-join (cross) │ │ │ ├── columns: x:3 - │ │ │ ├── internal-ordering: -3 - │ │ │ ├── project + │ │ │ ├── limit │ │ │ │ ├── columns: x:3 - │ │ │ │ └── scan xy - │ │ │ │ └── columns: x:3 y:4 rowid:5!null crdb_internal_mvcc_timestamp:6 tableoid:7 - │ │ │ └── const: 1 + │ │ │ │ ├── internal-ordering: -3 + │ │ │ │ ├── project + │ │ │ │ │ ├── columns: x:3 + │ │ │ │ │ └── scan xy + │ │ │ │ │ └── columns: x:3 y:4 rowid:5!null crdb_internal_mvcc_timestamp:6 tableoid:7 + │ │ │ │ └── const: 1 + │ │ │ ├── values + │ │ │ │ └── tuple + │ │ │ └── filters (true) │ │ └── projections │ │ └── variable: x:3 [as=i:26] │ └── projections @@ -4565,21 +4575,26 @@ project │ └── project │ ├── columns: "_stmt_exec_ret_6":23 │ ├── project - │ │ ├── columns: i:22!null - │ │ ├── limit - │ │ │ ├── columns: x:12!null - │ │ │ ├── internal-ordering: -12 - │ │ │ ├── project + │ │ ├── columns: i:22 + │ │ ├── right-join (cross) + │ │ │ ├── columns: x:12 + │ │ │ ├── limit │ │ │ │ ├── columns: x:12!null - │ │ │ │ └── select - │ │ │ │ ├── columns: x:12!null y:13 rowid:14!null crdb_internal_mvcc_timestamp:15 tableoid:16 - │ │ │ │ ├── scan xy - │ │ │ │ │ └── columns: x:12 y:13 rowid:14!null crdb_internal_mvcc_timestamp:15 tableoid:16 - │ │ │ │ └── filters - │ │ │ │ └── lt - │ │ │ │ ├── variable: x:12 - │ │ │ │ └── variable: i:11 - │ │ │ └── const: 1 + │ │ │ │ ├── internal-ordering: -12 + │ │ │ │ ├── project + │ │ │ │ │ ├── columns: x:12!null + │ │ │ │ │ └── select + │ │ │ │ │ ├── columns: x:12!null y:13 rowid:14!null crdb_internal_mvcc_timestamp:15 tableoid:16 + │ │ │ │ │ ├── scan xy + │ │ │ │ │ │ └── columns: x:12 y:13 rowid:14!null crdb_internal_mvcc_timestamp:15 tableoid:16 + │ │ │ │ │ └── filters + │ │ │ │ │ └── lt + │ │ │ │ │ ├── variable: x:12 + │ │ │ │ │ └── variable: i:11 + │ │ │ │ └── const: 1 + │ │ │ ├── values + │ │ │ │ └── tuple + │ │ │ └── filters (true) │ │ └── projections │ │ └── variable: x:12 [as=i:22] │ └── projections @@ -5762,3 +5777,200 @@ project │ └── tuple [as=stmt_return_1:1, type=tuple{bool}] │ └── false [type=bool] └── const: 1 [type=int] + +# Regression test for #114826 - ensure that SQL statements return at least one +# row. +exec-ddl +CREATE TABLE t114826 (a INT); +---- + +exec-ddl +CREATE FUNCTION f114826() RETURNS INT AS $$ + DECLARE + found INT; + BEGIN + RAISE NOTICE 'HERE 1'; + SELECT a INTO found FROM t114826 WHERE a = 3; + RAISE NOTICE 'HERE 2'; + INSERT INTO t114826 (SELECT * FROM t114826) RETURNING a INTO found; + RAISE NOTICE 'HERE 3'; + RETURN 10; + END +$$ LANGUAGE PLpgSQL; +---- + +build format=show-scalars +SELECT f114826(); +---- +project + ├── columns: f114826:35 + ├── values + │ └── tuple + └── projections + └── udf: f114826 [as=f114826:35] + └── body + └── limit + ├── columns: "_stmt_raise_1":34 + ├── project + │ ├── columns: "_stmt_raise_1":34 + │ ├── project + │ │ ├── columns: found:1 + │ │ ├── values + │ │ │ └── tuple + │ │ └── projections + │ │ └── cast: INT8 [as=found:1] + │ │ └── null + │ └── projections + │ └── udf: _stmt_raise_1 [as="_stmt_raise_1":34] + │ ├── args + │ │ └── variable: found:1 + │ ├── params: found:2 + │ └── body + │ ├── project + │ │ ├── columns: stmt_raise_2:3 + │ │ ├── values + │ │ │ └── tuple + │ │ └── projections + │ │ └── function: crdb_internal.plpgsql_raise [as=stmt_raise_2:3] + │ │ ├── const: 'NOTICE' + │ │ ├── const: 'HERE 1' + │ │ ├── const: '' + │ │ ├── const: '' + │ │ └── const: '00000' + │ └── project + │ ├── columns: "_stmt_exec_3":33 + │ ├── values + │ │ └── tuple + │ └── projections + │ └── udf: _stmt_exec_3 [as="_stmt_exec_3":33] + │ ├── args + │ │ └── variable: found:2 + │ ├── params: found:4 + │ └── body + │ └── project + │ ├── columns: "_stmt_exec_ret_4":32 + │ ├── project + │ │ ├── columns: found:31 + │ │ ├── right-join (cross) + │ │ │ ├── columns: a:5 + │ │ │ ├── limit + │ │ │ │ ├── columns: a:5!null + │ │ │ │ ├── project + │ │ │ │ │ ├── columns: a:5!null + │ │ │ │ │ └── select + │ │ │ │ │ ├── columns: a:5!null rowid:6!null crdb_internal_mvcc_timestamp:7 tableoid:8 + │ │ │ │ │ ├── scan t114826 + │ │ │ │ │ │ └── columns: a:5 rowid:6!null crdb_internal_mvcc_timestamp:7 tableoid:8 + │ │ │ │ │ └── filters + │ │ │ │ │ └── eq + │ │ │ │ │ ├── variable: a:5 + │ │ │ │ │ └── const: 3 + │ │ │ │ └── const: 1 + │ │ │ ├── values + │ │ │ │ └── tuple + │ │ │ └── filters (true) + │ │ └── projections + │ │ └── variable: a:5 [as=found:31] + │ └── projections + │ └── udf: _stmt_exec_ret_4 [as="_stmt_exec_ret_4":32] + │ ├── args + │ │ └── variable: found:31 + │ ├── params: found:9 + │ └── body + │ └── project + │ ├── columns: "_stmt_raise_5":30 + │ ├── values + │ │ └── tuple + │ └── projections + │ └── udf: _stmt_raise_5 [as="_stmt_raise_5":30] + │ ├── args + │ │ └── variable: found:9 + │ ├── params: found:10 + │ └── body + │ ├── project + │ │ ├── columns: stmt_raise_6:11 + │ │ ├── values + │ │ │ └── tuple + │ │ └── projections + │ │ └── function: crdb_internal.plpgsql_raise [as=stmt_raise_6:11] + │ │ ├── const: 'NOTICE' + │ │ ├── const: 'HERE 2' + │ │ ├── const: '' + │ │ ├── const: '' + │ │ └── const: '00000' + │ └── project + │ ├── columns: "_stmt_exec_7":29 + │ ├── values + │ │ └── tuple + │ └── projections + │ └── udf: _stmt_exec_7 [as="_stmt_exec_7":29] + │ ├── args + │ │ └── variable: found:10 + │ ├── params: found:12 + │ └── body + │ └── project + │ ├── columns: "_stmt_exec_ret_8":28 + │ ├── project + │ │ ├── columns: found:27 + │ │ ├── right-join (cross) + │ │ │ ├── columns: a:13 + │ │ │ ├── limit + │ │ │ │ ├── columns: a:13 + │ │ │ │ ├── project + │ │ │ │ │ ├── columns: a:13 + │ │ │ │ │ └── insert t114826 + │ │ │ │ │ ├── columns: a:13 rowid:14!null + │ │ │ │ │ ├── insert-mapping: + │ │ │ │ │ │ ├── a:17 => a:13 + │ │ │ │ │ │ └── rowid_default:21 => rowid:14 + │ │ │ │ │ ├── return-mapping: + │ │ │ │ │ │ ├── a:17 => a:13 + │ │ │ │ │ │ └── rowid_default:21 => rowid:14 + │ │ │ │ │ └── project + │ │ │ │ │ ├── columns: rowid_default:21 a:17 + │ │ │ │ │ ├── project + │ │ │ │ │ │ ├── columns: a:17 + │ │ │ │ │ │ └── scan t114826 + │ │ │ │ │ │ └── columns: a:17 rowid:18!null crdb_internal_mvcc_timestamp:19 tableoid:20 + │ │ │ │ │ └── projections + │ │ │ │ │ └── function: unique_rowid [as=rowid_default:21] + │ │ │ │ └── const: 1 + │ │ │ ├── values + │ │ │ │ └── tuple + │ │ │ └── filters (true) + │ │ └── projections + │ │ └── variable: a:13 [as=found:27] + │ └── projections + │ └── udf: _stmt_exec_ret_8 [as="_stmt_exec_ret_8":28] + │ ├── args + │ │ └── variable: found:27 + │ ├── params: found:22 + │ └── body + │ └── project + │ ├── columns: "_stmt_raise_9":26 + │ ├── values + │ │ └── tuple + │ └── projections + │ └── udf: _stmt_raise_9 [as="_stmt_raise_9":26] + │ ├── args + │ │ └── variable: found:22 + │ ├── params: found:23 + │ └── body + │ ├── project + │ │ ├── columns: stmt_raise_10:24 + │ │ ├── values + │ │ │ └── tuple + │ │ └── projections + │ │ └── function: crdb_internal.plpgsql_raise [as=stmt_raise_10:24] + │ │ ├── const: 'NOTICE' + │ │ ├── const: 'HERE 3' + │ │ ├── const: '' + │ │ ├── const: '' + │ │ └── const: '00000' + │ └── project + │ ├── columns: stmt_return_11:25!null + │ ├── values + │ │ └── tuple + │ └── projections + │ └── const: 10 [as=stmt_return_11:25] + └── const: 1