Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 312d21d

Browse files
committed
Update docs & tests to reflect that unassigned OLD/NEW are now NULL.
For a long time, plpgsql has allowed trigger functions to parse references to OLD and NEW even if the current trigger event type didn't assign a value to one or the other variable; but actually executing such a reference would fail. The v11 changes to use "expanded records" for DTYPE_REC variables changed the behavior so that the unassigned variable now reads as a null composite value. While this behavioral change was more or less unintentional, it seems that leaving it like this is better than adding code and complexity to be bug-compatible with the old way. The change doesn't break any code that worked before, and it eliminates a gotcha that often required extra code to work around. Hence, update the docs to say that these variables are "null" not "unassigned" when not relevant to the event type. And add a regression test covering the behavior, so that we'll notice if we ever break it again. Per report from Kristjan Tammekivi. Discussion: https://postgr.es/m/CAABK7uL-uC9ZxKBXzo_68pKt7cECfNRv+c35CXZpjq6jCAzYYA@mail.gmail.com
1 parent f2e14c2 commit 312d21d

File tree

6 files changed

+86
-9
lines changed

6 files changed

+86
-9
lines changed

doc/src/sgml/plpgsql.sgml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3857,7 +3857,7 @@ ASSERT <replaceable class="parameter">condition</replaceable> <optional> , <repl
38573857
<para>
38583858
Data type <type>RECORD</type>; variable holding the new
38593859
database row for <command>INSERT</command>/<command>UPDATE</command> operations in row-level
3860-
triggers. This variable is unassigned in statement-level triggers
3860+
triggers. This variable is null in statement-level triggers
38613861
and for <command>DELETE</command> operations.
38623862
</para>
38633863
</listitem>
@@ -3869,7 +3869,7 @@ ASSERT <replaceable class="parameter">condition</replaceable> <optional> , <repl
38693869
<para>
38703870
Data type <type>RECORD</type>; variable holding the old
38713871
database row for <command>UPDATE</command>/<command>DELETE</command> operations in row-level
3872-
triggers. This variable is unassigned in statement-level triggers
3872+
triggers. This variable is null in statement-level triggers
38733873
and for <command>INSERT</command> operations.
38743874
</para>
38753875
</listitem>

doc/src/sgml/release-11.sgml

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1033,6 +1033,23 @@ Branch: REL9_3_STABLE [84261eb10] 2018-10-19 17:02:26 -0400
10331033
</para>
10341034
</listitem>
10351035

1036+
<listitem>
1037+
<!--
1038+
2018-02-13 [4b93f5799] Make plpgsql use its DTYPE_REC code paths for composite-
1039+
-->
1040+
1041+
<para>
1042+
In PL/pgSQL trigger functions, the <varname>OLD</varname>
1043+
and <varname>NEW</varname> variables now read as NULL when not
1044+
assigned (Tom Lane)
1045+
</para>
1046+
1047+
<para>
1048+
Previously, references to these variables could be parsed but not
1049+
executed.
1050+
</para>
1051+
</listitem>
1052+
10361053
</itemizedlist>
10371054

10381055
</sect2>
@@ -2574,7 +2591,6 @@ same commits as above
25742591
<listitem>
25752592
<!--
25762593
2018-02-13 [4b93f5799] Make plpgsql use its DTYPE_REC code paths for composite-
2577-
25782594
-->
25792595

25802596
<para>

src/pl/plpgsql/src/Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ DATA = plpgsql.control plpgsql--1.0.sql plpgsql--unpackaged--1.0.sql
2727
REGRESS_OPTS = --dbname=$(PL_TESTDB)
2828

2929
REGRESS = plpgsql_call plpgsql_control plpgsql_domain plpgsql_record \
30-
plpgsql_cache plpgsql_transaction plpgsql_varprops
30+
plpgsql_cache plpgsql_transaction plpgsql_trigger plpgsql_varprops
3131

3232
all: all-lib
3333

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
-- Simple test to verify accessibility of the OLD and NEW trigger variables
2+
create table testtr (a int, b text);
3+
create function testtr_trigger() returns trigger language plpgsql as
4+
$$begin
5+
raise notice 'tg_op = %', tg_op;
6+
raise notice 'old(%) = %', old.a, row(old.*);
7+
raise notice 'new(%) = %', new.a, row(new.*);
8+
if (tg_op = 'DELETE') then
9+
return old;
10+
else
11+
return new;
12+
end if;
13+
end$$;
14+
create trigger testtr_trigger before insert or delete or update on testtr
15+
for each row execute function testtr_trigger();
16+
insert into testtr values (1, 'one'), (2, 'two');
17+
NOTICE: tg_op = INSERT
18+
NOTICE: old(<NULL>) = (,)
19+
NOTICE: new(1) = (1,one)
20+
NOTICE: tg_op = INSERT
21+
NOTICE: old(<NULL>) = (,)
22+
NOTICE: new(2) = (2,two)
23+
update testtr set a = a + 1;
24+
NOTICE: tg_op = UPDATE
25+
NOTICE: old(1) = (1,one)
26+
NOTICE: new(2) = (2,one)
27+
NOTICE: tg_op = UPDATE
28+
NOTICE: old(2) = (2,two)
29+
NOTICE: new(3) = (3,two)
30+
delete from testtr;
31+
NOTICE: tg_op = DELETE
32+
NOTICE: old(2) = (2,one)
33+
NOTICE: new(<NULL>) = (,)
34+
NOTICE: tg_op = DELETE
35+
NOTICE: old(3) = (3,two)
36+
NOTICE: new(<NULL>) = (,)

src/pl/plpgsql/src/pl_exec.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -890,11 +890,12 @@ plpgsql_exec_trigger(PLpgSQL_function *func,
890890
/*
891891
* Put the OLD and NEW tuples into record variables
892892
*
893-
* We make the tupdescs available in both records even though only one may
894-
* have a value. This allows parsing of record references to succeed in
895-
* functions that are used for multiple trigger types. For example, we
896-
* might have a test like "if (TG_OP = 'INSERT' and NEW.foo = 'xyz')",
897-
* which should parse regardless of the current trigger type.
893+
* We set up expanded records for both variables even though only one may
894+
* have a value. This allows record references to succeed in functions
895+
* that are used for multiple trigger types. For example, we might have a
896+
* test like "if (TG_OP = 'INSERT' and NEW.foo = 'xyz')", which should
897+
* work regardless of the current trigger type. If a value is actually
898+
* fetched from an unsupplied tuple, it will read as NULL.
898899
*/
899900
tupdesc = RelationGetDescr(trigdata->tg_relation);
900901

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
-- Simple test to verify accessibility of the OLD and NEW trigger variables
2+
3+
create table testtr (a int, b text);
4+
5+
create function testtr_trigger() returns trigger language plpgsql as
6+
$$begin
7+
raise notice 'tg_op = %', tg_op;
8+
raise notice 'old(%) = %', old.a, row(old.*);
9+
raise notice 'new(%) = %', new.a, row(new.*);
10+
if (tg_op = 'DELETE') then
11+
return old;
12+
else
13+
return new;
14+
end if;
15+
end$$;
16+
17+
create trigger testtr_trigger before insert or delete or update on testtr
18+
for each row execute function testtr_trigger();
19+
20+
insert into testtr values (1, 'one'), (2, 'two');
21+
22+
update testtr set a = a + 1;
23+
24+
delete from testtr;

0 commit comments

Comments
 (0)