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 
Reported-by: Phillip Menke 
Discussion: https://postgr.es/m/16794-350a655580fbb9ae@postgresql.org

diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 2aa24183def198639cdf5843b1f7b781577fdf5f..a04d02fa211729a73ed4f9afa38118e557269043 100644 (file)
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -4026,8 +4026,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 9361a1417b638ad1629ddf6fe8a25aaf8209225b..29d58c6be5162f398a098b95bc6d49649f24399f 100644 (file)
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -2726,16 +2726,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 f6610b0585b9bd33b495036bf4cf77aca5bcdfa9..cebeaf2e3adf1b0088354a1ac7b3ab8734d02aa5 100644 (file)
--- 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 9c3872eb14532d89def565a9bc3131689d72f657..53d89b0947802205a266f3462b55cd0d7e6937ab 100644 (file)
--- 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 $$