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

diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 02d2f42865172f7b99c13ebb44493bbab563eb86..1e9a4625cc631d5040366f2937d81e64c0a33ba7 100644 (file)
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -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>
 

diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 3e7086c5e52957f7223830dbd48112a7939e79a6..2d687f6dfb6bb8eb24cf689a35a701d5d1cf787d 100644 (file)
--- 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 1dc525251a9c667d70ee8e17d77bfd1b29087de4..b263002293bb8f1a50e4e16c597a03d73aff6e0b 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 bebe276ef432762ecc95757cde3e4108f2dc24a2..01f66af699cb4a7cc85d865148c020f2b5491474 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 $$