I happened to notice that there are a couple of places in plpgsql that will let you assign a new value to a variable that's marked CONSTANT:
* We don't complain if an output parameter in a CALL statement is constant. * We don't complain if a refcursor variable is constant, even though OPEN may assign a new value to it. The attached quick-hack patch closes both of these oversights. Perhaps the OPEN change is a little too aggressive, since if you give the refcursor variable some non-null initial value, OPEN won't change it; in that usage a CONSTANT marking could be allowed. But I really seriously doubt that anybody out there is marking such variables as constants, so I thought throwing the error at compile time was better than postponing it to runtime so we could handle that. Regardless of which way we handle that point, I'm inclined to change this only in HEAD. Probably people wouldn't thank us for making the back branches more strict. regards, tom lane PS: I didn't do it here, but I'm kind of tempted to pull out all the cursor-related tests in plpgsql.sql and move them to a new test file under src/pl/plpgsql/src/sql/. They look pretty self-contained, and I doubt they're worth keeping in the core tests.
diff --git a/src/pl/plpgsql/src/expected/plpgsql_call.out b/src/pl/plpgsql/src/expected/plpgsql_call.out index 7b156f3489..1ec6182a8d 100644 --- a/src/pl/plpgsql/src/expected/plpgsql_call.out +++ b/src/pl/plpgsql/src/expected/plpgsql_call.out @@ -122,6 +122,18 @@ CONTEXT: PL/pgSQL function inline_code_block line 6 at CALL 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; diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index ceb011810d..d523d13682 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 { @@ -8242,6 +8247,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/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y index 11e86c1609..62eb3f24e6 100644 --- a/src/pl/plpgsql/src/pl_gram.y +++ b/src/pl/plpgsql/src/pl_gram.y @@ -2098,6 +2098,12 @@ stmt_open : K_OPEN cursor_variable PLpgSQL_stmt_open *new; int tok; + /* + * cursor_variable must be assignable, since we might + * change its value at runtime + */ + check_assignable((PLpgSQL_datum *) $2, @2); + new = palloc0(sizeof(PLpgSQL_stmt_open)); new->cmd_type = PLPGSQL_STMT_OPEN; new->lineno = plpgsql_location_to_lineno(@1); diff --git a/src/pl/plpgsql/src/sql/plpgsql_call.sql b/src/pl/plpgsql/src/sql/plpgsql_call.sql index 8108d05060..5028398348 100644 --- a/src/pl/plpgsql/src/sql/plpgsql_call.sql +++ b/src/pl/plpgsql/src/sql/plpgsql_call.sql @@ -112,6 +112,18 @@ END; $$; +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 $$ diff --git a/src/test/regress/expected/plpgsql.out b/src/test/regress/expected/plpgsql.out index 33eb5e54b9..1af379d1d5 100644 --- a/src/test/regress/expected/plpgsql.out +++ b/src/test/regress/expected/plpgsql.out @@ -2256,6 +2256,18 @@ select refcursor_test2(20000, 20000) as "Should be false", f | t (1 row) +-- should fail to compile +create function return_constant_refcursor() returns refcursor as $$ +declare + rc constant refcursor; +begin + open rc for select a from rc_test; + return rc; +end +$$ language plpgsql; +ERROR: variable "rc" is declared CONSTANT +LINE 5: open rc for select a from rc_test; + ^ -- -- tests for cursors with named parameter arguments -- diff --git a/src/test/regress/sql/plpgsql.sql b/src/test/regress/sql/plpgsql.sql index bffb7b703b..da56450f0f 100644 --- a/src/test/regress/sql/plpgsql.sql +++ b/src/test/regress/sql/plpgsql.sql @@ -1933,6 +1933,16 @@ $$ language plpgsql; select refcursor_test2(20000, 20000) as "Should be false", refcursor_test2(20, 20) as "Should be true"; +-- should fail to compile +create function return_constant_refcursor() returns refcursor as $$ +declare + rc constant refcursor; +begin + open rc for select a from rc_test; + return rc; +end +$$ language plpgsql; + -- -- tests for cursors with named parameter arguments --