"Kevin Grittner" <kevin.gritt...@wicourts.gov> wrote:
 
> Attached is a patch based on these thoughts.
 
OK, really attached this time.
 
-Kevin

*** a/src/backend/executor/execMain.c
--- b/src/backend/executor/execMain.c
***************
*** 1921,1928 **** EvalPlanQualFetch(EState *estate, Relation relation, int 
lockmode,
                        switch (test)
                        {
                                case HeapTupleSelfUpdated:
-                                       /* treat it as deleted; do not process 
*/
                                        ReleaseBuffer(buffer);
                                        return NULL;
  
                                case HeapTupleMayBeUpdated:
--- 1921,1936 ----
                        switch (test)
                        {
                                case HeapTupleSelfUpdated:
                                        ReleaseBuffer(buffer);
+                                       if (!ItemPointerEquals(&update_ctid, 
&tuple.t_self))
+                                       {
+                                               /* it was updated, so look at 
the updated version */
+                                               tuple.t_self = update_ctid;
+                                               /* updated row should have xmin 
matching this xmax */
+                                               priorXmax = update_xmax;
+                                               continue;
+                                       }
+                                       /* treat it as deleted; do not process 
*/
                                        return NULL;
  
                                case HeapTupleMayBeUpdated:
*** a/src/backend/executor/nodeModifyTable.c
--- b/src/backend/executor/nodeModifyTable.c
***************
*** 354,359 **** ldelete:;
--- 354,398 ----
                switch (result)
                {
                        case HeapTupleSelfUpdated:
+                               /*
+                                * There is no sensible action to take if the 
BEFORE DELETE
+                                * trigger for a row issues an UPDATE for the 
same row, either
+                                * directly or by performing DML which fires 
other triggers
+                                * which do the update.  We don't want to 
discard the original
+                                * DELETE while keeping the triggered actions 
based on its
+                                * deletion; and it would be no better to allow 
the original
+                                * DELETE while discarding some of its 
triggered actions.
+                                * Updating the row which is being deleted 
risks losing some
+                                * information which might be important 
according to business
+                                * rules; so throwing an error is the only safe 
course.
+                                * 
+                                * We want to be careful not to trigger this 
for join/updates
+                                * which join to the same row more than once, 
so we check
+                                * whether the tuple was expired by a command 
other than the
+                                * one which initially fired the trigger.  If 
it is, the
+                                * current command ID must have changed, so do 
that check
+                                * first, because it is fast.  If it has 
changed, chase down
+                                * to the last version of the row to see if it 
was changed by
+                                * a subsequent command.
+                                */
+                               if (GetCurrentCommandId(false) != 
estate->es_output_cid)
+                               {
+                                       HeapTuple       copyTuple;
+ 
+                                       copyTuple = EvalPlanQualFetch(estate,
+                                                                               
                  resultRelationDesc,
+                                                                               
                  LockTupleExclusive,
+                                                                               
                  &update_ctid,
+                                                                               
                  update_xmax);
+                                       if (copyTuple == NULL
+                                               || 
HeapTupleHeaderGetCmax(copyTuple->t_data) != estate->es_output_cid)
+                                       {
+                                               ereport(ERROR,
+                                                               
(errcode(ERRCODE_TRIGGERED_DATA_CHANGE_VIOLATION),
+                                                                errmsg("cannot 
update or delete a row from its BEFORE DELETE trigger"),
+                                                                
errhint("Consider moving code to an AFTER DELETE trigger.")));
+                                       }
+                               }
                                /* already deleted by self; nothing to do */
                                return NULL;
  
***************
*** 570,575 **** lreplace:;
--- 609,651 ----
                switch (result)
                {
                        case HeapTupleSelfUpdated:
+                               /*
+                                * There is no sensible action to take if the 
BEFORE UPDATE
+                                * trigger for a row issues another UPDATE for 
the same row,
+                                * either directly or by performing DML which 
fires other
+                                * triggers which do the update.  We don't want 
to discard the
+                                * original UPDATE while keeping the triggered 
actions based
+                                * on its update; and it would be no better to 
allow the
+                                * original UPDATE while discarding some of its 
triggered
+                                * actions.
+                                * 
+                                * We want to be careful not to trigger this 
for join/updates
+                                * which join to the same row more than once, 
so we check
+                                * whether the tuple was expired by a command 
other than the
+                                * one which initially fired the trigger.  If 
it is, the
+                                * current command ID must have changed, so do 
that check
+                                * first, because it is fast.  If it has 
changed, chase down
+                                * to the last version of the row to see if it 
was changed by
+                                * a subsequent command.
+                                */
+                               if (GetCurrentCommandId(false) != 
estate->es_output_cid)
+                               {
+                                       HeapTuple       copyTuple;
+ 
+                                       copyTuple = EvalPlanQualFetch(estate,
+                                                                               
                  resultRelationDesc,
+                                                                               
                  LockTupleExclusive,
+                                                                               
                  &update_ctid,
+                                                                               
                  update_xmax);
+                                       if (copyTuple == NULL
+                                               || 
HeapTupleHeaderGetCmax(copyTuple->t_data) != estate->es_output_cid)
+                                       {
+                                               ereport(ERROR,
+                                                               
(errcode(ERRCODE_TRIGGERED_DATA_CHANGE_VIOLATION),
+                                                                errmsg("cannot 
update or delete a row from its BEFORE UPDATE trigger"),
+                                                                
errhint("Consider moving code to an AFTER UPDATE trigger.")));
+                                       }
+                               }
                                /* already deleted by self; nothing to do */
                                return NULL;
  
*** a/src/test/regress/expected/triggers.out
--- b/src/test/regress/expected/triggers.out
***************
*** 1443,1445 **** NOTICE:  drop cascades to 2 other objects
--- 1443,1566 ----
  DETAIL:  drop cascades to view city_view
  drop cascades to view european_city_view
  DROP TABLE country_table;
+ --
+ -- Test updates to rows during firing of BEFORE ROW triggers.
+ --
+ create table parent (aid int not null primary key,
+                      val1 text,
+                      val2 text,
+                      val3 text,
+                      val4 text,
+                      bcnt int not null default 0);
+ NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "parent_pkey" 
for table "parent"
+ create table child (bid int not null primary key,
+                     aid int not null,
+                     val1 text);
+ NOTICE:  CREATE TABLE / PRIMARY KEY will create implicit index "child_pkey" 
for table "child"
+ create function parent_upd_func()
+   returns trigger language plpgsql as
+ $$
+ begin
+   if old.val1 <> new.val1 then
+     new.val2 = new.val1;
+     delete from child where child.aid = new.aid and child.val1 = new.val1;
+   end if;
+   return new;
+ end;
+ $$;
+ create trigger parent_upd_trig before update On parent
+   for each row execute procedure parent_upd_func();
+ create function parent_del_func()
+   returns trigger language plpgsql as
+ $$
+ begin
+   delete from child where aid = old.aid;
+   return old;
+ end;
+ $$;
+ create trigger parent_del_trig before delete On parent
+   for each row execute procedure parent_del_func();
+ create function child_ins_func()
+   returns trigger language plpgsql as
+ $$
+ begin
+   update parent set bcnt = bcnt + 1 where aid = new.aid;
+   return new;
+ end;
+ $$;
+ create trigger child_ins_trig after insert on child
+   for each row execute procedure child_ins_func();
+ create function child_del_func()
+   returns trigger language plpgsql as
+ $$
+ begin
+   update parent set bcnt = bcnt - 1 where aid = old.aid;
+   return old;
+ end;
+ $$;
+ create trigger child_del_trig after delete on child
+   for each row execute procedure child_del_func();
+ insert into parent values (1, 'a', 'a', 'a', 'a', 0);
+ insert into child values (10, 1, 'b');
+ select * from parent; select * from child;
+  aid | val1 | val2 | val3 | val4 | bcnt 
+ -----+------+------+------+------+------
+    1 | a    | a    | a    | a    |    1
+ (1 row)
+ 
+  bid | aid | val1 
+ -----+-----+------
+   10 |   1 | b
+ (1 row)
+ 
+ update parent set val1 = 'b' where aid = 1;
+ ERROR:  cannot update or delete a row from its BEFORE UPDATE trigger
+ HINT:  Consider moving code to an AFTER UPDATE trigger.
+ select * from parent; select * from child;
+  aid | val1 | val2 | val3 | val4 | bcnt 
+ -----+------+------+------+------+------
+    1 | a    | a    | a    | a    |    1
+ (1 row)
+ 
+  bid | aid | val1 
+ -----+-----+------
+   10 |   1 | b
+ (1 row)
+ 
+ delete from parent where aid = 1;
+ ERROR:  cannot update or delete a row from its BEFORE DELETE trigger
+ HINT:  Consider moving code to an AFTER DELETE trigger.
+ select * from parent; select * from child;
+  aid | val1 | val2 | val3 | val4 | bcnt 
+ -----+------+------+------+------+------
+    1 | a    | a    | a    | a    |    1
+ (1 row)
+ 
+  bid | aid | val1 
+ -----+-----+------
+   10 |   1 | b
+ (1 row)
+ 
+ create or replace function parent_del_func()
+   returns trigger language plpgsql as
+ $$
+ begin
+   delete from child where aid = old.aid;
+   if found then
+     delete from parent where aid = old.aid;
+     return null;
+   end if;
+   return old;
+ end;
+ $$;
+ delete from parent where aid = 1;
+ select * from parent; select * from child;
+  aid | val1 | val2 | val3 | val4 | bcnt 
+ -----+------+------+------+------+------
+ (0 rows)
+ 
+  bid | aid | val1 
+ -----+-----+------
+ (0 rows)
+ 
+ drop table parent, child;
*** a/src/test/regress/sql/triggers.sql
--- b/src/test/regress/sql/triggers.sql
***************
*** 961,963 **** SELECT * FROM city_view;
--- 961,1050 ----
  
  DROP TABLE city_table CASCADE;
  DROP TABLE country_table;
+ 
+ --
+ -- Test updates to rows during firing of BEFORE ROW triggers.
+ --
+ 
+ create table parent (aid int not null primary key,
+                      val1 text,
+                      val2 text,
+                      val3 text,
+                      val4 text,
+                      bcnt int not null default 0);
+ create table child (bid int not null primary key,
+                     aid int not null,
+                     val1 text);
+ 
+ create function parent_upd_func()
+   returns trigger language plpgsql as
+ $$
+ begin
+   if old.val1 <> new.val1 then
+     new.val2 = new.val1;
+     delete from child where child.aid = new.aid and child.val1 = new.val1;
+   end if;
+   return new;
+ end;
+ $$;
+ create trigger parent_upd_trig before update On parent
+   for each row execute procedure parent_upd_func();
+ 
+ create function parent_del_func()
+   returns trigger language plpgsql as
+ $$
+ begin
+   delete from child where aid = old.aid;
+   return old;
+ end;
+ $$;
+ create trigger parent_del_trig before delete On parent
+   for each row execute procedure parent_del_func();
+ 
+ create function child_ins_func()
+   returns trigger language plpgsql as
+ $$
+ begin
+   update parent set bcnt = bcnt + 1 where aid = new.aid;
+   return new;
+ end;
+ $$;
+ create trigger child_ins_trig after insert on child
+   for each row execute procedure child_ins_func();
+ 
+ create function child_del_func()
+   returns trigger language plpgsql as
+ $$
+ begin
+   update parent set bcnt = bcnt - 1 where aid = old.aid;
+   return old;
+ end;
+ $$;
+ create trigger child_del_trig after delete on child
+   for each row execute procedure child_del_func();
+ 
+ insert into parent values (1, 'a', 'a', 'a', 'a', 0);
+ insert into child values (10, 1, 'b');
+ select * from parent; select * from child;
+ update parent set val1 = 'b' where aid = 1;
+ select * from parent; select * from child;
+ delete from parent where aid = 1;
+ select * from parent; select * from child;
+ 
+ create or replace function parent_del_func()
+   returns trigger language plpgsql as
+ $$
+ begin
+   delete from child where aid = old.aid;
+   if found then
+     delete from parent where aid = old.aid;
+     return null;
+   end if;
+   return old;
+ end;
+ $$;
+ 
+ delete from parent where aid = 1;
+ select * from parent; select * from child;
+ 
+ drop table parent, child;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to