Skip to content

Commit 6317609

Browse files
tglsfdctglsfdc
committed
Add control knobs for plpgsql's variable resolution behavior, and make the
default be "throw error on conflict", as per discussions. The GUC variable is plpgsql.variable_conflict, with values "error", "use_variable", "use_column". The behavior can also be specified per-function by inserting one of #variable_conflict error #variable_conflict use_variable #variable_conflict use_column at the start of the function body. The 8.5 release notes will need to mention using "use_variable" to retain backward-compatible behavior, although we should encourage people to migrate to the much less mistake-prone "error" setting. Update the plpgsql documentation to match this and other recent changes.
1 parent 01038d4 commit 6317609

File tree

9 files changed

+316
-215
lines changed

9 files changed

+316
-215
lines changed

‎doc/src/sgml/config.sgml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- $PostgreSQL: pgsql/doc/src/sgml/config.sgml,v 1.232 2009/10/21 20:38:58 tgl Exp $ -->
1+
<!-- $PostgreSQL: pgsql/doc/src/sgml/config.sgml,v 1.233 2009/11/13 22:43:39 tgl Exp $ -->
22

33
<chapter Id="runtime-config">
44
<title>Server Configuration</title>
@@ -5162,8 +5162,8 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
51625162
when using custom variables:
51635163

51645164
<programlisting>
5165-
custom_variable_classes = 'plr,plperl'
5166-
plr.path = '/usr/lib/R'
5165+
custom_variable_classes = 'plpgsql,plperl'
5166+
plpgsql.variable_conflict = use_variable
51675167
plperl.use_strict = true
51685168
plruby.use_strict = true # generates error: unknown class name
51695169
</programlisting>

‎doc/src/sgml/plpgsql.sgml

Lines changed: 186 additions & 199 deletions
Large diffs are not rendered by default.

‎src/pl/plpgsql/src/gram.y

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/pl/plpgsql/src/gram.y,v 1.135 2009/11/12 00:13:00 tgl Exp $
11+
* $PostgreSQL: pgsql/src/pl/plpgsql/src/gram.y,v 1.136 2009/11/13 22:43:40 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -256,6 +256,7 @@ static List *read_raise_options(void);
256256
%token <keyword> K_ELSIF
257257
%token <keyword> K_END
258258
%token <keyword> K_ERRCODE
259+
%token <keyword> K_ERROR
259260
%token <keyword> K_EXCEPTION
260261
%token <keyword> K_EXECUTE
261262
%token <keyword> K_EXIT
@@ -301,7 +302,10 @@ static List *read_raise_options(void);
301302
%token <keyword> K_THEN
302303
%token <keyword> K_TO
303304
%token <keyword> K_TYPE
305+
%token <keyword> K_USE_COLUMN
306+
%token <keyword> K_USE_VARIABLE
304307
%token <keyword> K_USING
308+
%token <keyword> K_VARIABLE_CONFLICT
305309
%token <keyword> K_WARNING
306310
%token <keyword> K_WHEN
307311
%token <keyword> K_WHILE
@@ -322,6 +326,18 @@ comp_option : '#' K_OPTION K_DUMP
322326
{
323327
plpgsql_DumpExecTree = true;
324328
}
329+
| '#' K_VARIABLE_CONFLICT K_ERROR
330+
{
331+
plpgsql_curr_compile->resolve_option = PLPGSQL_RESOLVE_ERROR;
332+
}
333+
| '#' K_VARIABLE_CONFLICT K_USE_VARIABLE
334+
{
335+
plpgsql_curr_compile->resolve_option = PLPGSQL_RESOLVE_VARIABLE;
336+
}
337+
| '#' K_VARIABLE_CONFLICT K_USE_COLUMN
338+
{
339+
plpgsql_curr_compile->resolve_option = PLPGSQL_RESOLVE_COLUMN;
340+
}
325341
;
326342

327343
opt_semi :
@@ -1969,6 +1985,7 @@ unreserved_keyword :
19691985
| K_DETAIL
19701986
| K_DUMP
19711987
| K_ERRCODE
1988+
| K_ERROR
19721989
| K_FIRST
19731990
| K_FORWARD
19741991
| K_HINT
@@ -1991,6 +2008,9 @@ unreserved_keyword :
19912008
| K_SCROLL
19922009
| K_SQLSTATE
19932010
| K_TYPE
2011+
| K_USE_COLUMN
2012+
| K_USE_VARIABLE
2013+
| K_VARIABLE_CONFLICT
19942014
| K_WARNING
19952015
;
19962016

‎src/pl/plpgsql/src/pl_comp.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_comp.c,v 1.145 2009/11/12 00:13:00 tgl Exp $
11+
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_comp.c,v 1.146 2009/11/13 22:43:42 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -351,7 +351,7 @@ do_compile(FunctionCallInfo fcinfo,
351351
function->fn_is_trigger = is_trigger;
352352
function->fn_cxt = func_cxt;
353353
function->out_param_varno = -1; /* set up for no OUT param */
354-
function->resolve_option = PLPGSQL_RESOLVE_BEFORE;
354+
function->resolve_option = plpgsql_variable_conflict;
355355

356356
/*
357357
* Initialize the compiler, particularly the namespace stack. The
@@ -782,7 +782,7 @@ plpgsql_compile_inline(char *proc_source)
782782
function->fn_is_trigger = false;
783783
function->fn_cxt = func_cxt;
784784
function->out_param_varno = -1; /* set up for no OUT param */
785-
function->resolve_option = PLPGSQL_RESOLVE_BEFORE;
785+
function->resolve_option = plpgsql_variable_conflict;
786786

787787
plpgsql_ns_init();
788788
plpgsql_ns_push(func_name);
@@ -948,7 +948,7 @@ plpgsql_pre_column_ref(ParseState *pstate, ColumnRef *cref)
948948
{
949949
PLpgSQL_expr *expr = (PLpgSQL_expr *) pstate->p_ref_hook_state;
950950

951-
if (expr->func->resolve_option == PLPGSQL_RESOLVE_BEFORE)
951+
if (expr->func->resolve_option == PLPGSQL_RESOLVE_VARIABLE)
952952
return resolve_column_ref(expr, cref);
953953
else
954954
return NULL;
@@ -963,10 +963,10 @@ plpgsql_post_column_ref(ParseState *pstate, ColumnRef *cref, Node *var)
963963
PLpgSQL_expr *expr = (PLpgSQL_expr *) pstate->p_ref_hook_state;
964964
Node *myvar;
965965

966-
if (expr->func->resolve_option == PLPGSQL_RESOLVE_BEFORE)
966+
if (expr->func->resolve_option == PLPGSQL_RESOLVE_VARIABLE)
967967
return NULL; /* we already found there's no match */
968968

969-
if (expr->func->resolve_option == PLPGSQL_RESOLVE_AFTER && var != NULL)
969+
if (expr->func->resolve_option == PLPGSQL_RESOLVE_COLUMN && var != NULL)
970970
return NULL; /* there's a table column, prefer that */
971971

972972
myvar = resolve_column_ref(expr, cref);

‎src/pl/plpgsql/src/pl_handler.c

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_handler.c,v 1.47 2009/11/04 22:26:07 tgl Exp $
11+
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_handler.c,v 1.48 2009/11/13 22:43:42 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -26,6 +26,17 @@
2626

2727
PG_MODULE_MAGIC;
2828

29+
/* Custom GUC variable */
30+
static const struct config_enum_entry variable_conflict_options[] = {
31+
{"error", PLPGSQL_RESOLVE_ERROR, false},
32+
{"use_variable", PLPGSQL_RESOLVE_VARIABLE, false},
33+
{"use_column", PLPGSQL_RESOLVE_COLUMN, false},
34+
{NULL, 0, false}
35+
};
36+
37+
int plpgsql_variable_conflict = PLPGSQL_RESOLVE_ERROR;
38+
39+
/* Hook for plugins */
2940
PLpgSQL_plugin **plugin_ptr = NULL;
3041

3142

@@ -45,6 +56,17 @@ _PG_init(void)
4556

4657
pg_bindtextdomain(TEXTDOMAIN);
4758

59+
DefineCustomEnumVariable("plpgsql.variable_conflict",
60+
gettext_noop("Sets handling of conflicts between PL/pgSQL variable names and table column names."),
61+
NULL,
62+
&plpgsql_variable_conflict,
63+
PLPGSQL_RESOLVE_ERROR,
64+
variable_conflict_options,
65+
PGC_SUSET, 0,
66+
NULL, NULL);
67+
68+
EmitWarningsOnPlaceholders("plpgsql");
69+
4870
plpgsql_HashTableInit();
4971
RegisterXactCallback(plpgsql_xact_cb, NULL);
5072
RegisterSubXactCallback(plpgsql_subxact_cb, NULL);

‎src/pl/plpgsql/src/pl_scanner.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*
1010
*
1111
* IDENTIFICATION
12-
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_scanner.c,v 1.1 2009/11/12 00:13:00 tgl Exp $
12+
* $PostgreSQL: pgsql/src/pl/plpgsql/src/pl_scanner.c,v 1.2 2009/11/13 22:43:42 tgl Exp $
1313
*
1414
*-------------------------------------------------------------------------
1515
*/
@@ -113,6 +113,7 @@ static const ScanKeyword unreserved_keywords[] = {
113113
PG_KEYWORD("detail", K_DETAIL, UNRESERVED_KEYWORD)
114114
PG_KEYWORD("dump", K_DUMP, UNRESERVED_KEYWORD)
115115
PG_KEYWORD("errcode", K_ERRCODE, UNRESERVED_KEYWORD)
116+
PG_KEYWORD("error", K_ERROR, UNRESERVED_KEYWORD)
116117
PG_KEYWORD("first", K_FIRST, UNRESERVED_KEYWORD)
117118
PG_KEYWORD("forward", K_FORWARD, UNRESERVED_KEYWORD)
118119
PG_KEYWORD("hint", K_HINT, UNRESERVED_KEYWORD)
@@ -135,6 +136,9 @@ static const ScanKeyword unreserved_keywords[] = {
135136
PG_KEYWORD("scroll", K_SCROLL, UNRESERVED_KEYWORD)
136137
PG_KEYWORD("sqlstate", K_SQLSTATE, UNRESERVED_KEYWORD)
137138
PG_KEYWORD("type", K_TYPE, UNRESERVED_KEYWORD)
139+
PG_KEYWORD("use_column", K_USE_COLUMN, UNRESERVED_KEYWORD)
140+
PG_KEYWORD("use_variable", K_USE_VARIABLE, UNRESERVED_KEYWORD)
141+
PG_KEYWORD("variable_conflict", K_VARIABLE_CONFLICT, UNRESERVED_KEYWORD)
138142
PG_KEYWORD("warning", K_WARNING, UNRESERVED_KEYWORD)
139143
};
140144

‎src/pl/plpgsql/src/plpgsql.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/pl/plpgsql/src/plpgsql.h,v 1.124 2009/11/12 00:13:00 tgl Exp $
11+
* $PostgreSQL: pgsql/src/pl/plpgsql/src/plpgsql.h,v 1.125 2009/11/13 22:43:42 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -147,9 +147,9 @@ enum
147147
*/
148148
typedef enum
149149
{
150-
PLPGSQL_RESOLVE_BEFORE, /* prefer plpgsql var to table column */
151-
PLPGSQL_RESOLVE_AFTER, /* prefer table column to plpgsql var */
152-
PLPGSQL_RESOLVE_ERROR /* throw error if ambiguous */
150+
PLPGSQL_RESOLVE_ERROR, /* throw error if ambiguous */
151+
PLPGSQL_RESOLVE_VARIABLE, /* prefer plpgsql var to table column */
152+
PLPGSQL_RESOLVE_COLUMN /* prefer table column to plpgsql var */
153153
} PLpgSQL_resolve_option;
154154

155155

@@ -794,6 +794,8 @@ typedef struct
794794
* Global variable declarations
795795
**********************************************************************/
796796

797+
extern int plpgsql_variable_conflict;
798+
797799
extern bool plpgsql_check_syntax;
798800
extern bool plpgsql_DumpExecTree;
799801
extern bool plpgsql_LookupIdentifiers;

‎src/test/regress/expected/plpgsql.out

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4004,6 +4004,7 @@ select scope_test();
40044004

40054005
drop function scope_test();
40064006
-- Check handling of conflicts between plpgsql vars and table columns.
4007+
set plpgsql.variable_conflict = error;
40074008
create function conflict_test() returns setof int8_tbl as $$
40084009
declare r record;
40094010
q1 bigint := 42;
@@ -4013,6 +4014,23 @@ begin
40134014
end loop;
40144015
end;
40154016
$$ language plpgsql;
4017+
select * from conflict_test();
4018+
ERROR: column reference "q1" is ambiguous
4019+
LINE 1: select q1,q2 from int8_tbl
4020+
^
4021+
DETAIL: It could refer to either a PL/pgSQL variable or a table column.
4022+
QUERY: select q1,q2 from int8_tbl
4023+
CONTEXT: PL/pgSQL function "conflict_test" line 4 at FOR over SELECT rows
4024+
create or replace function conflict_test() returns setof int8_tbl as $$
4025+
#variable_conflict use_variable
4026+
declare r record;
4027+
q1 bigint := 42;
4028+
begin
4029+
for r in select q1,q2 from int8_tbl loop
4030+
return next r;
4031+
end loop;
4032+
end;
4033+
$$ language plpgsql;
40164034
select * from conflict_test();
40174035
q1 | q2
40184036
----+-------------------
@@ -4023,6 +4041,26 @@ select * from conflict_test();
40234041
42 | -4567890123456789
40244042
(5 rows)
40254043

4044+
create or replace function conflict_test() returns setof int8_tbl as $$
4045+
#variable_conflict use_column
4046+
declare r record;
4047+
q1 bigint := 42;
4048+
begin
4049+
for r in select q1,q2 from int8_tbl loop
4050+
return next r;
4051+
end loop;
4052+
end;
4053+
$$ language plpgsql;
4054+
select * from conflict_test();
4055+
q1 | q2
4056+
------------------+-------------------
4057+
123 | 456
4058+
123 | 4567890123456789
4059+
4567890123456789 | 123
4060+
4567890123456789 | 4567890123456789
4061+
4567890123456789 | -4567890123456789
4062+
(5 rows)
4063+
40264064
drop function conflict_test();
40274065
-- Check that an unreserved keyword can be used as a variable name
40284066
create function unreserved_test() returns int as $$

‎src/test/regress/sql/plpgsql.sql

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3176,6 +3176,8 @@ drop function scope_test();
31763176
31773177
-- Check handling of conflicts between plpgsql vars and table columns.
31783178
3179+
set plpgsql.variable_conflict = error;
3180+
31793181
create function conflict_test() returns setof int8_tbl as $$
31803182
declare r record;
31813183
q1 bigint := 42;
@@ -3188,6 +3190,32 @@ $$ language plpgsql;
31883190
31893191
select * from conflict_test();
31903192
3193+
create or replace function conflict_test() returns setof int8_tbl as $$
3194+
#variable_conflict use_variable
3195+
declare r record;
3196+
q1 bigint := 42;
3197+
begin
3198+
for r in select q1,q2 from int8_tbl loop
3199+
return next r;
3200+
end loop;
3201+
end;
3202+
$$ language plpgsql;
3203+
3204+
select * from conflict_test();
3205+
3206+
create or replace function conflict_test() returns setof int8_tbl as $$
3207+
#variable_conflict use_column
3208+
declare r record;
3209+
q1 bigint := 42;
3210+
begin
3211+
for r in select q1,q2 from int8_tbl loop
3212+
return next r;
3213+
end loop;
3214+
end;
3215+
$$ language plpgsql;
3216+
3217+
select * from conflict_test();
3218+
31913219
drop function conflict_test();
31923220
31933221
-- Check that an unreserved keyword can be used as a variable name

0 commit comments

Comments
 (0)