From 6f5c8a8ec23f8ab00da4d2b77bfc8af2a578c4d3 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Thu, 28 Jan 2021 16:56:07 -0300 Subject: [PATCH] Remove bogus restriction from BEFORE UPDATE triggers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In trying to protect the user from inconsistent behavior, commit 487e9861d0cf "Enable BEFORE row-level triggers for partitioned tables" tried to prevent BEFORE UPDATE FOR EACH ROW triggers from moving the row from one partition to another. However, it turns out that the restriction is wrong in two ways: first, it fails spuriously, preventing valid situations from working, as in bug #16794; and second, they don't protect from any misbehavior, because tuple routing would cope anyway. Fix by removing that restriction. We keep the same restriction on BEFORE INSERT FOR EACH ROW triggers, though. It is valid and useful there. In the future we could remove it by having tuple reroute work for inserts as it does for updates. Backpatch to 13. Author: Álvaro Herrera Reported-by: Phillip Menke Discussion: https://postgr.es/m/16794-350a655580fbb9ae@postgresql.org --- doc/src/sgml/ddl.sgml | 4 +- src/backend/commands/trigger.c | 10 ----- src/test/regress/expected/triggers.out | 57 ++++++++++++++++++++++---- src/test/regress/sql/triggers.sql | 28 ++++++++++++- 4 files changed, 77 insertions(+), 22 deletions(-) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 02d2f42865..1e9a4625cc 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -4027,8 +4027,8 @@ ALTER INDEX measurement_city_id_logdate_key - BEFORE ROW triggers cannot change which partition - is the final destination for a new row. + BEFORE ROW triggers on INSERT + cannot change which partition is the final destination for a new row. diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 3e7086c5e5..2d687f6dfb 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -2799,16 +2799,6 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate, { ExecForceStoreHeapTuple(newtuple, newslot, false); - if (trigger->tgisclone && - !ExecPartitionCheck(relinfo, newslot, estate, false)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("moving row to another partition during a BEFORE trigger is not supported"), - errdetail("Before executing trigger \"%s\", the row was to be in partition \"%s.%s\".", - trigger->tgname, - get_namespace_name(RelationGetNamespace(relinfo->ri_RelationDesc)), - RelationGetRelationName(relinfo->ri_RelationDesc)))); - /* * If the tuple returned by the trigger / being stored, is the old * row version, and the heap tuple passed to the trigger was diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out index 1dc525251a..b263002293 100644 --- a/src/test/regress/expected/triggers.out +++ b/src/test/regress/expected/triggers.out @@ -2359,8 +2359,8 @@ insert into parted values (1, 1, 'uno uno v2'); -- fail ERROR: moving row to another partition during a BEFORE FOR EACH ROW trigger is not supported DETAIL: Before executing trigger "t", the row was to be in partition "public.parted_1_1". update parted set c = c || 'v3'; -- fail -ERROR: moving row to another partition during a BEFORE trigger is not supported -DETAIL: Before executing trigger "t", the row was to be in partition "public.parted_1_1". +ERROR: no partition of relation "parted" found for row +DETAIL: Partition key of the failing row contains (a) = (2). create or replace function parted_trigfunc() returns trigger language plpgsql as $$ begin new.b = new.b + 1; @@ -2371,23 +2371,62 @@ insert into parted values (1, 1, 'uno uno v4'); -- fail ERROR: moving row to another partition during a BEFORE FOR EACH ROW trigger is not supported DETAIL: Before executing trigger "t", the row was to be in partition "public.parted_1_1". update parted set c = c || 'v5'; -- fail -ERROR: moving row to another partition during a BEFORE trigger is not supported -DETAIL: Before executing trigger "t", the row was to be in partition "public.parted_1_1". +ERROR: no partition of relation "parted_1" found for row +DETAIL: Partition key of the failing row contains (b) = (2). create or replace function parted_trigfunc() returns trigger language plpgsql as $$ begin - new.c = new.c || ' and so'; + new.c = new.c || ' did '|| TG_OP; return new; end; $$; insert into parted values (1, 1, 'uno uno'); -- works update parted set c = c || ' v6'; -- works select tableoid::regclass, * from parted; - tableoid | a | b | c -------------+---+---+-------------------------- - parted_1_1 | 1 | 1 | uno uno v1 v6 and so - parted_1_1 | 1 | 1 | uno uno and so v6 and so + tableoid | a | b | c +------------+---+---+---------------------------------- + parted_1_1 | 1 | 1 | uno uno v1 v6 did UPDATE + parted_1_1 | 1 | 1 | uno uno did INSERT v6 did UPDATE (2 rows) +-- update itself moves tuple to new partition; trigger still works +truncate table parted; +create table parted_2 partition of parted for values in (2); +insert into parted values (1, 1, 'uno uno v5'); +update parted set a = 2; +select tableoid::regclass, * from parted; + tableoid | a | b | c +----------+---+---+--------------------------------------------- + parted_2 | 2 | 1 | uno uno v5 did INSERT did UPDATE did INSERT +(1 row) + +-- both trigger and update change the partition +create or replace function parted_trigfunc2() returns trigger language plpgsql as $$ +begin + new.a = new.a + 1; + return new; +end; +$$; +create trigger t2 before update on parted + for each row execute function parted_trigfunc2(); +truncate table parted; +insert into parted values (1, 1, 'uno uno v6'); +create table parted_3 partition of parted for values in (3); +update parted set a = a + 1; +select tableoid::regclass, * from parted; + tableoid | a | b | c +----------+---+---+--------------------------------------------- + parted_3 | 3 | 1 | uno uno v6 did INSERT did UPDATE did INSERT +(1 row) + +-- there's no partition for a=0, but this update works anyway because +-- the trigger causes the tuple to be routed to another partition +update parted set a = 0; +select tableoid::regclass, * from parted; + tableoid | a | b | c +------------+---+---+------------------------------------------------------------------- + parted_1_1 | 1 | 1 | uno uno v6 did INSERT did UPDATE did INSERT did UPDATE did INSERT +(1 row) + drop table parted; create table parted (a int, b int, c text) partition by list ((a + b)); create or replace function parted_trigfunc() returns trigger language plpgsql as $$ diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql index bebe276ef4..01f66af699 100644 --- a/src/test/regress/sql/triggers.sql +++ b/src/test/regress/sql/triggers.sql @@ -1633,7 +1633,7 @@ insert into parted values (1, 1, 'uno uno v4'); -- fail update parted set c = c || 'v5'; -- fail create or replace function parted_trigfunc() returns trigger language plpgsql as $$ begin - new.c = new.c || ' and so'; + new.c = new.c || ' did '|| TG_OP; return new; end; $$; @@ -1641,6 +1641,32 @@ insert into parted values (1, 1, 'uno uno'); -- works update parted set c = c || ' v6'; -- works select tableoid::regclass, * from parted; +-- update itself moves tuple to new partition; trigger still works +truncate table parted; +create table parted_2 partition of parted for values in (2); +insert into parted values (1, 1, 'uno uno v5'); +update parted set a = 2; +select tableoid::regclass, * from parted; + +-- both trigger and update change the partition +create or replace function parted_trigfunc2() returns trigger language plpgsql as $$ +begin + new.a = new.a + 1; + return new; +end; +$$; +create trigger t2 before update on parted + for each row execute function parted_trigfunc2(); +truncate table parted; +insert into parted values (1, 1, 'uno uno v6'); +create table parted_3 partition of parted for values in (3); +update parted set a = a + 1; +select tableoid::regclass, * from parted; +-- there's no partition for a=0, but this update works anyway because +-- the trigger causes the tuple to be routed to another partition +update parted set a = 0; +select tableoid::regclass, * from parted; + drop table parted; create table parted (a int, b int, c text) partition by list ((a + b)); create or replace function parted_trigfunc() returns trigger language plpgsql as $$