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
This commit is contained in:
Tom Lane 2019-01-09 11:35:14 -05:00
parent a2b22d8e80
commit 59029b6fb7
6 changed files with 86 additions and 9 deletions

View File

@ -3849,7 +3849,7 @@ ASSERT <replaceable class="parameter">condition</replaceable> <optional> , <repl
<para>
Data type <type>RECORD</type>; variable holding the new
database row for <command>INSERT</command>/<command>UPDATE</command> operations in row-level
triggers. This variable is unassigned in statement-level triggers
triggers. This variable is null in statement-level triggers
and for <command>DELETE</command> operations.
</para>
</listitem>
@ -3861,7 +3861,7 @@ ASSERT <replaceable class="parameter">condition</replaceable> <optional> , <repl
<para>
Data type <type>RECORD</type>; variable holding the old
database row for <command>UPDATE</command>/<command>DELETE</command> operations in row-level
triggers. This variable is unassigned in statement-level triggers
triggers. This variable is null in statement-level triggers
and for <command>INSERT</command> operations.
</para>
</listitem>

View File

@ -1033,6 +1033,23 @@ Branch: REL9_3_STABLE [84261eb10] 2018-10-19 17:02:26 -0400
</para>
</listitem>
<listitem>
<!--
2018-02-13 [4b93f5799] Make plpgsql use its DTYPE_REC code paths for composite-
-->
<para>
In PL/pgSQL trigger functions, the <varname>OLD</varname>
and <varname>NEW</varname> variables now read as NULL when not
assigned (Tom Lane)
</para>
<para>
Previously, references to these variables could be parsed but not
executed.
</para>
</listitem>
</itemizedlist>
</sect2>
@ -2574,7 +2591,6 @@ same commits as above
<listitem>
<!--
2018-02-13 [4b93f5799] Make plpgsql use its DTYPE_REC code paths for composite-
-->
<para>

View File

@ -27,7 +27,7 @@ DATA = plpgsql.control plpgsql--1.0.sql plpgsql--unpackaged--1.0.sql
REGRESS_OPTS = --dbname=$(PL_TESTDB)
REGRESS = plpgsql_call plpgsql_control plpgsql_domain plpgsql_record \
plpgsql_cache plpgsql_transaction plpgsql_varprops
plpgsql_cache plpgsql_transaction plpgsql_trigger plpgsql_varprops
GEN_KEYWORDLIST = $(top_srcdir)/src/tools/gen_keywordlist.pl

View File

@ -0,0 +1,36 @@
-- Simple test to verify accessibility of the OLD and NEW trigger variables
create table testtr (a int, b text);
create function testtr_trigger() returns trigger language plpgsql as
$$begin
raise notice 'tg_op = %', tg_op;
raise notice 'old(%) = %', old.a, row(old.*);
raise notice 'new(%) = %', new.a, row(new.*);
if (tg_op = 'DELETE') then
return old;
else
return new;
end if;
end$$;
create trigger testtr_trigger before insert or delete or update on testtr
for each row execute function testtr_trigger();
insert into testtr values (1, 'one'), (2, 'two');
NOTICE: tg_op = INSERT
NOTICE: old(<NULL>) = (,)
NOTICE: new(1) = (1,one)
NOTICE: tg_op = INSERT
NOTICE: old(<NULL>) = (,)
NOTICE: new(2) = (2,two)
update testtr set a = a + 1;
NOTICE: tg_op = UPDATE
NOTICE: old(1) = (1,one)
NOTICE: new(2) = (2,one)
NOTICE: tg_op = UPDATE
NOTICE: old(2) = (2,two)
NOTICE: new(3) = (3,two)
delete from testtr;
NOTICE: tg_op = DELETE
NOTICE: old(2) = (2,one)
NOTICE: new(<NULL>) = (,)
NOTICE: tg_op = DELETE
NOTICE: old(3) = (3,two)
NOTICE: new(<NULL>) = (,)

View File

@ -891,11 +891,12 @@ plpgsql_exec_trigger(PLpgSQL_function *func,
/*
* Put the OLD and NEW tuples into record variables
*
* We make the tupdescs available in both records even though only one may
* have a value. This allows parsing of record references to succeed in
* functions that are used for multiple trigger types. For example, we
* might have a test like "if (TG_OP = 'INSERT' and NEW.foo = 'xyz')",
* which should parse regardless of the current trigger type.
* We set up expanded records for both variables even though only one may
* have a value. This allows record references to succeed in functions
* that are used for multiple trigger types. For example, we might have a
* test like "if (TG_OP = 'INSERT' and NEW.foo = 'xyz')", which should
* work regardless of the current trigger type. If a value is actually
* fetched from an unsupplied tuple, it will read as NULL.
*/
tupdesc = RelationGetDescr(trigdata->tg_relation);

View File

@ -0,0 +1,24 @@
-- Simple test to verify accessibility of the OLD and NEW trigger variables
create table testtr (a int, b text);
create function testtr_trigger() returns trigger language plpgsql as
$$begin
raise notice 'tg_op = %', tg_op;
raise notice 'old(%) = %', old.a, row(old.*);
raise notice 'new(%) = %', new.a, row(new.*);
if (tg_op = 'DELETE') then
return old;
else
return new;
end if;
end$$;
create trigger testtr_trigger before insert or delete or update on testtr
for each row execute function testtr_trigger();
insert into testtr values (1, 'one'), (2, 'two');
update testtr set a = a + 1;
delete from testtr;