Enable BEFORE row-level triggers for partitioned tables
authorAlvaro Herrera <alvherre@alvh.no-ip.org>
Wed, 18 Mar 2020 21:58:05 +0000 (18:58 -0300)
committerAlvaro Herrera <alvherre@alvh.no-ip.org>
Wed, 18 Mar 2020 21:58:05 +0000 (18:58 -0300)
... with the limitation that the tuple must remain in the same
partition.

Reviewed-by: Ashutosh Bapat
Discussion: https://postgr.es/m/20200227165158.GA2071@alvherre.pgsql

doc/src/sgml/ref/create_trigger.sgml
doc/src/sgml/trigger.sgml
src/backend/commands/tablecmds.c
src/backend/commands/trigger.c
src/backend/partitioning/partdesc.c
src/include/utils/reltrigger.h
src/test/regress/expected/triggers.out
src/test/regress/sql/triggers.sql

index 3b8f25ea4a5d788a72b2f5283629743ddc577917..bde3a63f90f15210bbfc089b437b3984992ac2a2 100644 (file)
@@ -526,7 +526,7 @@ UPDATE OF <replaceable>column_name1</replaceable> [, <replaceable>column_name2</
    Creating a row-level trigger on a partitioned table will cause identical
    triggers to be created in all its existing partitions; and any partitions
    created or attached later will contain an identical trigger, too.
-   Triggers on partitioned tables may only be <literal>AFTER</literal>.
+   Triggers on partitioned tables may not be <literal>INSTEAD OF</literal>.
   </para>
 
   <para>
index 7d9ad4763aa05c74c35f0fb85290b68b48f7f0cd..4a0e74652f7bf6b060c1e36340eff10414f30310 100644 (file)
     operated on, while row-level <literal>AFTER</literal> triggers fire at the end of
     the statement (but before any statement-level <literal>AFTER</literal> triggers).
     These types of triggers may only be defined on tables and
-    foreign tables, not views; <literal>BEFORE</literal> row-level triggers may not
-    be defined on partitioned tables.
+    foreign tables, not views.
     <literal>INSTEAD OF</literal> triggers may only be
     defined on views, and only at row level; they fire immediately as each
     row in the view is identified as needing to be operated on.
index 8c33b67c1b5ace10d51bf760deed5955280dfa47..729025470dc1160a3cf4a6ab9468fb6fdcabe8a7 100644 (file)
@@ -16546,7 +16546,8 @@ CloneRowTriggersToPartition(Relation parent, Relation partition)
                /*
                 * Complain if we find an unexpected trigger type.
                 */
-               if (!TRIGGER_FOR_AFTER(trigForm->tgtype))
+               if (!TRIGGER_FOR_BEFORE(trigForm->tgtype) &&
+                       !TRIGGER_FOR_AFTER(trigForm->tgtype))
                        elog(ERROR, "unexpected trigger \"%s\" found",
                                 NameStr(trigForm->tgname));
 
index 513427edf146ae5102cafad9e40eb5b6f2132d25..ed551ab73aad50b12a0b1c00a2fac0624ab96877 100644 (file)
@@ -221,18 +221,6 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString,
                 */
                if (stmt->row)
                {
-                       /*
-                        * BEFORE triggers FOR EACH ROW are forbidden, because they would
-                        * allow the user to direct the row to another partition, which
-                        * isn't implemented in the executor.
-                        */
-                       if (stmt->timing != TRIGGER_TYPE_AFTER)
-                               ereport(ERROR,
-                                               (errcode(ERRCODE_WRONG_OBJECT_TYPE),
-                                                errmsg("\"%s\" is a partitioned table",
-                                                               RelationGetRelationName(rel)),
-                                                errdetail("Partitioned tables cannot have BEFORE / FOR EACH ROW triggers.")));
-
                        /*
                         * Disallow use of transition tables.
                         *
@@ -1658,6 +1646,7 @@ RelationBuildTriggers(Relation relation)
                build->tgtype = pg_trigger->tgtype;
                build->tgenabled = pg_trigger->tgenabled;
                build->tgisinternal = pg_trigger->tgisinternal;
+               build->tgisclone = OidIsValid(pg_trigger->tgparentid);
                build->tgconstrrelid = pg_trigger->tgconstrrelid;
                build->tgconstrindid = pg_trigger->tgconstrindid;
                build->tgconstraint = pg_trigger->tgconstraint;
@@ -1961,6 +1950,8 @@ equalTriggerDescs(TriggerDesc *trigdesc1, TriggerDesc *trigdesc2)
                                return false;
                        if (trig1->tgisinternal != trig2->tgisinternal)
                                return false;
+                       if (trig1->tgisclone != trig2->tgisclone)
+                               return false;
                        if (trig1->tgconstrrelid != trig2->tgconstrrelid)
                                return false;
                        if (trig1->tgconstrindid != trig2->tgconstrindid)
@@ -2247,6 +2238,21 @@ ExecBRInsertTriggers(EState *estate, ResultRelInfo *relinfo,
                {
                        ExecForceStoreHeapTuple(newtuple, slot, false);
 
+                       /*
+                        * After a tuple in a partition goes through a trigger, the user
+                        * could have changed the partition key enough that the tuple
+                        * no longer fits the partition.  Verify that.
+                        */
+                       if (trigger->tgisclone &&
+                               !ExecPartitionCheck(relinfo, slot, estate, false))
+                               ereport(ERROR,
+                                               (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                                errmsg("moving row to another partition during a BEFORE FOR EACH ROW 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 (should_free)
                                heap_freetuple(oldtuple);
 
@@ -2741,6 +2747,16 @@ 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
index bdda42e40f35ababf7ec4b27c28a8bb3d0b17b8b..e7f23542e8027ae456886cc959216bffd23f9b88 100644 (file)
@@ -307,7 +307,7 @@ CreatePartitionDirectory(MemoryContext mcxt)
  *
  * The purpose of this function is to ensure that we get the same
  * PartitionDesc for each relation every time we look it up.  In the
- * face of current DDL, different PartitionDescs may be constructed with
+ * face of concurrent DDL, different PartitionDescs may be constructed with
  * different views of the catalog state, but any single particular OID
  * will always get the same PartitionDesc for as long as the same
  * PartitionDirectory is used.
index 28df43d83350fdbfbdae5e2857ad47a8e0f0a015..b22acb034e9e789d5274d832de9210fa05af6636 100644 (file)
@@ -29,6 +29,7 @@ typedef struct Trigger
        int16           tgtype;
        char            tgenabled;
        bool            tgisinternal;
+       bool            tgisclone;
        Oid                     tgconstrrelid;
        Oid                     tgconstrindid;
        Oid                     tgconstraint;
index 22e65cc1ecedd4503e5de10bf97294efdc7009c9..e9da4ef983e1c0819886cf5182fa85a32c465fcd 100644 (file)
@@ -1958,10 +1958,6 @@ drop table my_table;
 create table parted_trig (a int) partition by list (a);
 create function trigger_nothing() returns trigger
   language plpgsql as $$ begin end; $$;
-create trigger failed before insert or update or delete on parted_trig
-  for each row execute procedure trigger_nothing();
-ERROR:  "parted_trig" is a partitioned table
-DETAIL:  Partitioned tables cannot have BEFORE / FOR EACH ROW triggers.
 create trigger failed instead of update on parted_trig
   for each row execute procedure trigger_nothing();
 ERROR:  "parted_trig" is a table
@@ -2246,6 +2242,80 @@ NOTICE:  aasvogel <- woof!
 NOTICE:  trigger parted_trig on parted1_irreg AFTER INSERT for ROW: (a,b)=(3,aasvogel)
 NOTICE:  trigger parted_trig_odd on parted1_irreg AFTER INSERT for ROW: (a,b)=(3,aasvogel)
 drop table parted_irreg_ancestor;
+-- Before triggers and partitions
+create table parted (a int, b int, c text) partition by list (a);
+create table parted_1 partition of parted for values in (1)
+  partition by list (b);
+create table parted_1_1 partition of parted_1 for values in (1);
+create function parted_trigfunc() returns trigger language plpgsql as $$
+begin
+  new.a = new.a + 1;
+  return new;
+end;
+$$;
+insert into parted values (1, 1, 'uno uno v1');    -- works
+create trigger t before insert or update or delete on parted
+  for each row execute function parted_trigfunc();
+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".
+create or replace function parted_trigfunc() returns trigger language plpgsql as $$
+begin
+  new.b = new.b + 1;
+  return new;
+end;
+$$;
+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".
+create or replace function parted_trigfunc() returns trigger language plpgsql as $$
+begin
+  new.c = new.c || ' and so';
+  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
+(2 rows)
+
+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 $$
+begin
+  new.a = new.a + new.b;
+  return new;
+end;
+$$;
+create table parted_1 partition of parted for values in (1, 2);
+create table parted_2 partition of parted for values in (3, 4);
+create trigger t before insert or update on parted
+  for each row execute function parted_trigfunc();
+insert into parted values (0, 1, 'zero win');
+insert into parted values (1, 1, 'one 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".
+insert into parted values (1, 2, 'two 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_2".
+select * from parted;
+ a | b |    c     
+---+---+----------
+ 1 | 1 | zero win
+(1 row)
+
+drop table parted;
+drop function parted_trigfunc();
 --
 -- Constraint triggers and partitioned tables
 create table parted_constr_ancestor (a int, b text)
index 0f61fdf0ea2c13c9819ace3f389e774db1cbc1cf..80ffbb4b02810465839f1f64ba9c30c66e21b33a 100644 (file)
@@ -1348,8 +1348,6 @@ drop table my_table;
 create table parted_trig (a int) partition by list (a);
 create function trigger_nothing() returns trigger
   language plpgsql as $$ begin end; $$;
-create trigger failed before insert or update or delete on parted_trig
-  for each row execute procedure trigger_nothing();
 create trigger failed instead of update on parted_trig
   for each row execute procedure trigger_nothing();
 create trigger failed after update on parted_trig
@@ -1561,6 +1559,59 @@ insert into parted1_irreg values ('aardwolf', 2);
 insert into parted_irreg_ancestor values ('aasvogel', 3);
 drop table parted_irreg_ancestor;
 
+-- Before triggers and partitions
+create table parted (a int, b int, c text) partition by list (a);
+create table parted_1 partition of parted for values in (1)
+  partition by list (b);
+create table parted_1_1 partition of parted_1 for values in (1);
+create function parted_trigfunc() returns trigger language plpgsql as $$
+begin
+  new.a = new.a + 1;
+  return new;
+end;
+$$;
+insert into parted values (1, 1, 'uno uno v1');    -- works
+create trigger t before insert or update or delete on parted
+  for each row execute function parted_trigfunc();
+insert into parted values (1, 1, 'uno uno v2');    -- fail
+update parted set c = c || 'v3';                   -- fail
+create or replace function parted_trigfunc() returns trigger language plpgsql as $$
+begin
+  new.b = new.b + 1;
+  return new;
+end;
+$$;
+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';
+  return new;
+end;
+$$;
+insert into parted values (1, 1, 'uno uno');       -- works
+update parted set c = c || ' v6';                   -- works
+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 $$
+begin
+  new.a = new.a + new.b;
+  return new;
+end;
+$$;
+create table parted_1 partition of parted for values in (1, 2);
+create table parted_2 partition of parted for values in (3, 4);
+create trigger t before insert or update on parted
+  for each row execute function parted_trigfunc();
+insert into parted values (0, 1, 'zero win');
+insert into parted values (1, 1, 'one fail');
+insert into parted values (1, 2, 'two fail');
+select * from parted;
+drop table parted;
+drop function parted_trigfunc();
+
 --
 -- Constraint triggers and partitioned tables
 create table parted_constr_ancestor (a int, b text)