Remove bogus restriction from BEFORE UPDATE triggers

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 <alvherre@alvh.no-ip.org>
Reported-by: Phillip Menke <pg@pmenke.de>
Discussion: https://postgr.es/m/16794-350a655580fbb9ae@postgresql.org
This commit is contained in:
Alvaro Herrera 2021-01-28 16:56:07 -03:00
parent 1d9351a87c
commit 6f5c8a8ec2
No known key found for this signature in database
GPG Key ID: 1C20ACB9D5C564AE
4 changed files with 77 additions and 22 deletions

View File

@ -4027,8 +4027,8 @@ ALTER INDEX measurement_city_id_logdate_key
<listitem>
<para>
<literal>BEFORE ROW</literal> triggers cannot change which partition
is the final destination for a new row.
<literal>BEFORE ROW</literal> triggers on <literal>INSERT</literal>
cannot change which partition is the final destination for a new row.
</para>
</listitem>

View File

@ -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

View File

@ -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 $$

View File

@ -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 $$