diff options
| -rw-r--r-- | src/pl/plpgsql/src/expected/plpgsql_call.out | 12 | ||||
| -rw-r--r-- | src/pl/plpgsql/src/pl_exec.c | 63 | ||||
| -rw-r--r-- | src/pl/plpgsql/src/sql/plpgsql_call.sql | 12 | ||||
| -rw-r--r-- | src/test/regress/expected/plpgsql.out | 27 | ||||
| -rw-r--r-- | src/test/regress/sql/plpgsql.sql | 24 |
5 files changed, 134 insertions, 4 deletions
diff --git a/src/pl/plpgsql/src/expected/plpgsql_call.out b/src/pl/plpgsql/src/expected/plpgsql_call.out index 7b156f34893..1ec6182a8da 100644 --- a/src/pl/plpgsql/src/expected/plpgsql_call.out +++ b/src/pl/plpgsql/src/expected/plpgsql_call.out @@ -123,6 +123,18 @@ DO LANGUAGE plpgsql $$ DECLARE + x constant int := 3; + y int := 4; +BEGIN + CALL test_proc6(2, x, y); -- error because x is constant +END; +$$; +ERROR: variable "x" is declared CONSTANT +CONTEXT: PL/pgSQL function inline_code_block line 6 at CALL +DO +LANGUAGE plpgsql +$$ +DECLARE x int := 3; y int := 4; BEGIN diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index ceb011810d3..7bd2a9fff1c 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -331,6 +331,7 @@ static void exec_prepare_plan(PLpgSQL_execstate *estate, static void exec_simple_check_plan(PLpgSQL_execstate *estate, PLpgSQL_expr *expr); static void exec_save_simple_expr(PLpgSQL_expr *expr, CachedPlan *cplan); static void exec_check_rw_parameter(PLpgSQL_expr *expr); +static void exec_check_assignable(PLpgSQL_execstate *estate, int dno); static bool exec_eval_simple_expr(PLpgSQL_execstate *estate, PLpgSQL_expr *expr, Datum *result, @@ -2342,9 +2343,13 @@ make_callstmt_target(PLpgSQL_execstate *estate, PLpgSQL_expr *expr) if (IsA(n, Param)) { Param *param = (Param *) n; + int dno; /* paramid is offset by 1 (see make_datum_param()) */ - row->varnos[nfields++] = param->paramid - 1; + dno = param->paramid - 1; + /* must check assignability now, because grammar can't */ + exec_check_assignable(estate, dno); + row->varnos[nfields++] = dno; } else { @@ -2926,10 +2931,14 @@ exec_stmt_forc(PLpgSQL_execstate *estate, PLpgSQL_stmt_forc *stmt) SPI_result_code_string(SPI_result)); /* - * If cursor variable was NULL, store the generated portal name in it + * If cursor variable was NULL, store the generated portal name in it, + * after verifying it's okay to assign to. */ if (curname == NULL) + { + exec_check_assignable(estate, stmt->curvar); assign_text_var(estate, curvar, portal->name); + } /* * Clean up before entering exec_for_query @@ -4688,13 +4697,18 @@ exec_stmt_open(PLpgSQL_execstate *estate, PLpgSQL_stmt_open *stmt) stmt->cursor_options); /* - * If cursor variable was NULL, store the generated portal name in it. + * If cursor variable was NULL, store the generated portal name in it, + * after verifying it's okay to assign to. + * * Note: exec_dynquery_with_params already reset the stmt_mcontext, so * curname is a dangling pointer here; but testing it for nullness is * OK. */ if (curname == NULL) + { + exec_check_assignable(estate, stmt->curvar); assign_text_var(estate, curvar, portal->name); + } return PLPGSQL_RC_OK; } @@ -4763,10 +4777,14 @@ exec_stmt_open(PLpgSQL_execstate *estate, PLpgSQL_stmt_open *stmt) SPI_result_code_string(SPI_result)); /* - * If cursor variable was NULL, store the generated portal name in it + * If cursor variable was NULL, store the generated portal name in it, + * after verifying it's okay to assign to. */ if (curname == NULL) + { + exec_check_assignable(estate, stmt->curvar); assign_text_var(estate, curvar, portal->name); + } /* If we had any transient data, clean it up */ exec_eval_cleanup(estate); @@ -8242,6 +8260,43 @@ exec_check_rw_parameter(PLpgSQL_expr *expr) } } +/* + * exec_check_assignable --- is it OK to assign to the indicated datum? + * + * This should match pl_gram.y's check_assignable(). + */ +static void +exec_check_assignable(PLpgSQL_execstate *estate, int dno) +{ + PLpgSQL_datum *datum; + + Assert(dno >= 0 && dno < estate->ndatums); + datum = estate->datums[dno]; + switch (datum->dtype) + { + case PLPGSQL_DTYPE_VAR: + case PLPGSQL_DTYPE_PROMISE: + case PLPGSQL_DTYPE_REC: + if (((PLpgSQL_variable *) datum)->isconst) + ereport(ERROR, + (errcode(ERRCODE_ERROR_IN_ASSIGNMENT), + errmsg("variable \"%s\" is declared CONSTANT", + ((PLpgSQL_variable *) datum)->refname))); + break; + case PLPGSQL_DTYPE_ROW: + /* always assignable; member vars were checked at compile time */ + break; + case PLPGSQL_DTYPE_RECFIELD: + /* assignable if parent record is */ + exec_check_assignable(estate, + ((PLpgSQL_recfield *) datum)->recparentno); + break; + default: + elog(ERROR, "unrecognized dtype: %d", datum->dtype); + break; + } +} + /* ---------- * exec_set_found Set the global found variable to true/false * ---------- diff --git a/src/pl/plpgsql/src/sql/plpgsql_call.sql b/src/pl/plpgsql/src/sql/plpgsql_call.sql index 8108d050609..50283983480 100644 --- a/src/pl/plpgsql/src/sql/plpgsql_call.sql +++ b/src/pl/plpgsql/src/sql/plpgsql_call.sql @@ -116,6 +116,18 @@ DO LANGUAGE plpgsql $$ DECLARE + x constant int := 3; + y int := 4; +BEGIN + CALL test_proc6(2, x, y); -- error because x is constant +END; +$$; + + +DO +LANGUAGE plpgsql +$$ +DECLARE x int := 3; y int := 4; BEGIN diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index 33eb5e54b9c..08e42f17dc2 100644 --- a/src/test/regress/expected/plpgsql.out +++ b/src/test/regress/expected/plpgsql.out @@ -2256,6 +2256,33 @@ select refcursor_test2(20000, 20000) as "Should be false", f | t (1 row) +-- should fail +create function constant_refcursor() returns refcursor as $$ +declare + rc constant refcursor; +begin + open rc for select a from rc_test; + return rc; +end +$$ language plpgsql; +select constant_refcursor(); +ERROR: variable "rc" is declared CONSTANT +CONTEXT: PL/pgSQL function constant_refcursor() line 5 at OPEN +-- but it's okay like this +create or replace function constant_refcursor() returns refcursor as $$ +declare + rc constant refcursor := 'my_cursor_name'; +begin + open rc for select a from rc_test; + return rc; +end +$$ language plpgsql; +select constant_refcursor(); + constant_refcursor +-------------------- + my_cursor_name +(1 row) + -- -- tests for cursors with named parameter arguments -- diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql index bffb7b703b4..588c3310337 100644 --- a/src/test/regress/sql/plpgsql.sql +++ b/src/test/regress/sql/plpgsql.sql @@ -1933,6 +1933,30 @@ $$ language plpgsql; select refcursor_test2(20000, 20000) as "Should be false", refcursor_test2(20, 20) as "Should be true"; +-- should fail +create function constant_refcursor() returns refcursor as $$ +declare + rc constant refcursor; +begin + open rc for select a from rc_test; + return rc; +end +$$ language plpgsql; + +select constant_refcursor(); + +-- but it's okay like this +create or replace function constant_refcursor() returns refcursor as $$ +declare + rc constant refcursor := 'my_cursor_name'; +begin + open rc for select a from rc_test; + return rc; +end +$$ language plpgsql; + +select constant_refcursor(); + -- -- tests for cursors with named parameter arguments -- |