From 59029b6fb71e187c150d12f4c37bb2bf982d3125 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 9 Jan 2019 11:35:14 -0500 Subject: [PATCH] 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 --- doc/src/sgml/plpgsql.sgml | 4 +-- doc/src/sgml/release-11.sgml | 18 +++++++++- src/pl/plpgsql/src/Makefile | 2 +- .../plpgsql/src/expected/plpgsql_trigger.out | 36 +++++++++++++++++++ src/pl/plpgsql/src/pl_exec.c | 11 +++--- src/pl/plpgsql/src/sql/plpgsql_trigger.sql | 24 +++++++++++++ 6 files changed, 86 insertions(+), 9 deletions(-) create mode 100644 src/pl/plpgsql/src/expected/plpgsql_trigger.out create mode 100644 src/pl/plpgsql/src/sql/plpgsql_trigger.sql diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml index 1f2abbb5d1..f8c6435c50 100644 --- a/doc/src/sgml/plpgsql.sgml +++ b/doc/src/sgml/plpgsql.sgml @@ -3849,7 +3849,7 @@ ASSERT condition , Data type RECORD; variable holding the new database row for INSERT/UPDATE operations in row-level - triggers. This variable is unassigned in statement-level triggers + triggers. This variable is null in statement-level triggers and for DELETE operations. @@ -3861,7 +3861,7 @@ ASSERT condition , Data type RECORD; variable holding the old database row for UPDATE/DELETE operations in row-level - triggers. This variable is unassigned in statement-level triggers + triggers. This variable is null in statement-level triggers and for INSERT operations. diff --git a/doc/src/sgml/release-11.sgml b/doc/src/sgml/release-11.sgml index f35b0d8cc9..b129d32264 100644 --- a/doc/src/sgml/release-11.sgml +++ b/doc/src/sgml/release-11.sgml @@ -1033,6 +1033,23 @@ Branch: REL9_3_STABLE [84261eb10] 2018-10-19 17:02:26 -0400 + + + + + In PL/pgSQL trigger functions, the OLD + and NEW variables now read as NULL when not + assigned (Tom Lane) + + + + Previously, references to these variables could be parsed but not + executed. + + + @@ -2574,7 +2591,6 @@ same commits as above diff --git a/src/pl/plpgsql/src/Makefile b/src/pl/plpgsql/src/Makefile index 9dd4a74c34..f5958d1267 100644 --- a/src/pl/plpgsql/src/Makefile +++ b/src/pl/plpgsql/src/Makefile @@ -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 diff --git a/src/pl/plpgsql/src/expected/plpgsql_trigger.out b/src/pl/plpgsql/src/expected/plpgsql_trigger.out new file mode 100644 index 0000000000..3cc67badba --- /dev/null +++ b/src/pl/plpgsql/src/expected/plpgsql_trigger.out @@ -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() = (,) +NOTICE: new(1) = (1,one) +NOTICE: tg_op = INSERT +NOTICE: old() = (,) +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() = (,) +NOTICE: tg_op = DELETE +NOTICE: old(3) = (3,two) +NOTICE: new() = (,) diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index 92e7ec4c60..5c6dbe4c5f 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -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); diff --git a/src/pl/plpgsql/src/sql/plpgsql_trigger.sql b/src/pl/plpgsql/src/sql/plpgsql_trigger.sql new file mode 100644 index 0000000000..e04c273c51 --- /dev/null +++ b/src/pl/plpgsql/src/sql/plpgsql_trigger.sql @@ -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;