diff options
author | Noah Misch <noah@leadboat.com> | 2014-03-23 02:16:34 -0400 |
---|---|---|
committer | Noah Misch <noah@leadboat.com> | 2014-03-23 02:16:34 -0400 |
commit | 7cbe57c34dec4860243e6d0f81738cfbb6e5d069 (patch) | |
tree | 1b2e725b85caef56f986db8ae7c43732819c1f5c | |
parent | 6115480c543c0141011a99db78987ad13540be59 (diff) | |
download | postgresql-7cbe57c34dec4860243e6d0f81738cfbb6e5d069.tar.gz postgresql-7cbe57c34dec4860243e6d0f81738cfbb6e5d069.zip |
Offer triggers on foreign tables.
This covers all the SQL-standard trigger types supported for regular
tables; it does not cover constraint triggers. The approach for
acquiring the old row mirrors that for view INSTEAD OF triggers. For
AFTER ROW triggers, we spool the foreign tuples to a tuplestore.
This changes the FDW API contract; when deciding which columns to
populate in the slot returned from data modification callbacks, writable
FDWs will need to check for AFTER ROW triggers in addition to checking
for a RETURNING clause.
In support of the feature addition, refactor the TriggerFlags bits and
the assembly of old tuples in ModifyTable.
Ronan Dunklau, reviewed by KaiGai Kohei; some additional hacking by me.
-rw-r--r-- | contrib/postgres_fdw/deparse.c | 63 | ||||
-rw-r--r-- | contrib/postgres_fdw/expected/postgres_fdw.out | 319 | ||||
-rw-r--r-- | contrib/postgres_fdw/postgres_fdw.c | 4 | ||||
-rw-r--r-- | contrib/postgres_fdw/sql/postgres_fdw.sql | 216 | ||||
-rw-r--r-- | doc/src/sgml/fdwhandler.sgml | 36 | ||||
-rw-r--r-- | doc/src/sgml/ref/create_trigger.sgml | 43 | ||||
-rw-r--r-- | doc/src/sgml/trigger.sgml | 25 | ||||
-rw-r--r-- | src/backend/commands/tablecmds.c | 3 | ||||
-rw-r--r-- | src/backend/commands/trigger.c | 423 | ||||
-rw-r--r-- | src/backend/executor/nodeModifyTable.c | 90 | ||||
-rw-r--r-- | src/backend/rewrite/rewriteHandler.c | 36 | ||||
-rw-r--r-- | src/include/commands/trigger.h | 8 | ||||
-rw-r--r-- | src/test/regress/expected/foreign_data.out | 37 | ||||
-rw-r--r-- | src/test/regress/sql/foreign_data.sql | 44 |
14 files changed, 1145 insertions, 202 deletions
diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c index 2dfe80da0af..32c01350714 100644 --- a/contrib/postgres_fdw/deparse.c +++ b/contrib/postgres_fdw/deparse.c @@ -110,6 +110,7 @@ static void deparseTargetList(StringInfo buf, List **retrieved_attrs); static void deparseReturningList(StringInfo buf, PlannerInfo *root, Index rtindex, Relation rel, + bool trig_after_row, List *returningList, List **retrieved_attrs); static void deparseColumnRef(StringInfo buf, int varno, int varattno, @@ -875,11 +876,9 @@ deparseInsertSql(StringInfo buf, PlannerInfo *root, else appendStringInfoString(buf, " DEFAULT VALUES"); - if (returningList) - deparseReturningList(buf, root, rtindex, rel, returningList, - retrieved_attrs); - else - *retrieved_attrs = NIL; + deparseReturningList(buf, root, rtindex, rel, + rel->trigdesc && rel->trigdesc->trig_insert_after_row, + returningList, retrieved_attrs); } /* @@ -919,11 +918,9 @@ deparseUpdateSql(StringInfo buf, PlannerInfo *root, } appendStringInfoString(buf, " WHERE ctid = $1"); - if (returningList) - deparseReturningList(buf, root, rtindex, rel, returningList, - retrieved_attrs); - else - *retrieved_attrs = NIL; + deparseReturningList(buf, root, rtindex, rel, + rel->trigdesc && rel->trigdesc->trig_update_after_row, + returningList, retrieved_attrs); } /* @@ -943,34 +940,48 @@ deparseDeleteSql(StringInfo buf, PlannerInfo *root, deparseRelation(buf, rel); appendStringInfoString(buf, " WHERE ctid = $1"); - if (returningList) - deparseReturningList(buf, root, rtindex, rel, returningList, - retrieved_attrs); - else - *retrieved_attrs = NIL; + deparseReturningList(buf, root, rtindex, rel, + rel->trigdesc && rel->trigdesc->trig_delete_after_row, + returningList, retrieved_attrs); } /* - * deparse RETURNING clause of INSERT/UPDATE/DELETE + * Add a RETURNING clause, if needed, to an INSERT/UPDATE/DELETE. */ static void deparseReturningList(StringInfo buf, PlannerInfo *root, Index rtindex, Relation rel, + bool trig_after_row, List *returningList, List **retrieved_attrs) { - Bitmapset *attrs_used; + Bitmapset *attrs_used = NULL; - /* - * We need the attrs mentioned in the query's RETURNING list. - */ - attrs_used = NULL; - pull_varattnos((Node *) returningList, rtindex, - &attrs_used); + if (trig_after_row) + { + /* whole-row reference acquires all non-system columns */ + attrs_used = + bms_make_singleton(0 - FirstLowInvalidHeapAttributeNumber); + } - appendStringInfoString(buf, " RETURNING "); - deparseTargetList(buf, root, rtindex, rel, attrs_used, - retrieved_attrs); + if (returningList != NIL) + { + /* + * We need the attrs, non-system and system, mentioned in the local + * query's RETURNING list. + */ + pull_varattnos((Node *) returningList, rtindex, + &attrs_used); + } + + if (attrs_used != NULL) + { + appendStringInfoString(buf, " RETURNING "); + deparseTargetList(buf, root, rtindex, rel, attrs_used, + retrieved_attrs); + } + else + *retrieved_attrs = NIL; } /* diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 9a3d6516672..671c329c920 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -2496,3 +2496,322 @@ select * from rem1; 11 | bye remote (4 rows) +-- =================================================================== +-- test local triggers +-- =================================================================== +-- Trigger functions "borrowed" from triggers regress test. +CREATE FUNCTION trigger_func() RETURNS trigger LANGUAGE plpgsql AS $$ +BEGIN + RAISE NOTICE 'trigger_func(%) called: action = %, when = %, level = %', + TG_ARGV[0], TG_OP, TG_WHEN, TG_LEVEL; + RETURN NULL; +END;$$; +CREATE TRIGGER trig_stmt_before BEFORE DELETE OR INSERT OR UPDATE ON rem1 + FOR EACH STATEMENT EXECUTE PROCEDURE trigger_func(); +CREATE TRIGGER trig_stmt_after AFTER DELETE OR INSERT OR UPDATE ON rem1 + FOR EACH STATEMENT EXECUTE PROCEDURE trigger_func(); +CREATE OR REPLACE FUNCTION trigger_data() RETURNS trigger +LANGUAGE plpgsql AS $$ + +declare + oldnew text[]; + relid text; + argstr text; +begin + + relid := TG_relid::regclass; + argstr := ''; + for i in 0 .. TG_nargs - 1 loop + if i > 0 then + argstr := argstr || ', '; + end if; + argstr := argstr || TG_argv[i]; + end loop; + + RAISE NOTICE '%(%) % % % ON %', + tg_name, argstr, TG_when, TG_level, TG_OP, relid; + oldnew := '{}'::text[]; + if TG_OP != 'INSERT' then + oldnew := array_append(oldnew, format('OLD: %s', OLD)); + end if; + + if TG_OP != 'DELETE' then + oldnew := array_append(oldnew, format('NEW: %s', NEW)); + end if; + + RAISE NOTICE '%', array_to_string(oldnew, ','); + + if TG_OP = 'DELETE' then + return OLD; + else + return NEW; + end if; +end; +$$; +-- Test basic functionality +CREATE TRIGGER trig_row_before +BEFORE INSERT OR UPDATE OR DELETE ON rem1 +FOR EACH ROW EXECUTE PROCEDURE trigger_data(23,'skidoo'); +CREATE TRIGGER trig_row_after +AFTER INSERT OR UPDATE OR DELETE ON rem1 +FOR EACH ROW EXECUTE PROCEDURE trigger_data(23,'skidoo'); +delete from rem1; +NOTICE: trigger_func(<NULL>) called: action = DELETE, when = BEFORE, level = STATEMENT +NOTICE: trig_row_before(23, skidoo) BEFORE ROW DELETE ON rem1 +NOTICE: OLD: (1,hi) +NOTICE: trig_row_before(23, skidoo) BEFORE ROW DELETE ON rem1 +NOTICE: OLD: (10,"hi remote") +NOTICE: trig_row_before(23, skidoo) BEFORE ROW DELETE ON rem1 +NOTICE: OLD: (2,bye) +NOTICE: trig_row_before(23, skidoo) BEFORE ROW DELETE ON rem1 +NOTICE: OLD: (11,"bye remote") +NOTICE: trig_row_after(23, skidoo) AFTER ROW DELETE ON rem1 +NOTICE: OLD: (1,hi) +NOTICE: trig_row_after(23, skidoo) AFTER ROW DELETE ON rem1 +NOTICE: OLD: (10,"hi remote") +NOTICE: trig_row_after(23, skidoo) AFTER ROW DELETE ON rem1 +NOTICE: OLD: (2,bye) +NOTICE: trig_row_after(23, skidoo) AFTER ROW DELETE ON rem1 +NOTICE: OLD: (11,"bye remote") +NOTICE: trigger_func(<NULL>) called: action = DELETE, when = AFTER, level = STATEMENT +insert into rem1 values(1,'insert'); +NOTICE: trigger_func(<NULL>) called: action = INSERT, when = BEFORE, level = STATEMENT +NOTICE: trig_row_before(23, skidoo) BEFORE ROW INSERT ON rem1 +NOTICE: NEW: (1,insert) +NOTICE: trig_row_after(23, skidoo) AFTER ROW INSERT ON rem1 +NOTICE: NEW: (1,insert) +NOTICE: trigger_func(<NULL>) called: action = INSERT, when = AFTER, level = STATEMENT +update rem1 set f2 = 'update' where f1 = 1; +NOTICE: trigger_func(<NULL>) called: action = UPDATE, when = BEFORE, level = STATEMENT +NOTICE: trig_row_before(23, skidoo) BEFORE ROW UPDATE ON rem1 +NOTICE: OLD: (1,insert),NEW: (1,update) +NOTICE: trig_row_after(23, skidoo) AFTER ROW UPDATE ON rem1 +NOTICE: OLD: (1,insert),NEW: (1,update) +NOTICE: trigger_func(<NULL>) called: action = UPDATE, when = AFTER, level = STATEMENT +update rem1 set f2 = f2 || f2; +NOTICE: trigger_func(<NULL>) called: action = UPDATE, when = BEFORE, level = STATEMENT +NOTICE: trig_row_before(23, skidoo) BEFORE ROW UPDATE ON rem1 +NOTICE: OLD: (1,update),NEW: (1,updateupdate) +NOTICE: trig_row_after(23, skidoo) AFTER ROW UPDATE ON rem1 +NOTICE: OLD: (1,update),NEW: (1,updateupdate) +NOTICE: trigger_func(<NULL>) called: action = UPDATE, when = AFTER, level = STATEMENT +-- cleanup +DROP TRIGGER trig_row_before ON rem1; +DROP TRIGGER trig_row_after ON rem1; +DROP TRIGGER trig_stmt_before ON rem1; +DROP TRIGGER trig_stmt_after ON rem1; +DELETE from rem1; +-- Test WHEN conditions +CREATE TRIGGER trig_row_before_insupd +BEFORE INSERT OR UPDATE ON rem1 +FOR EACH ROW +WHEN (NEW.f2 like '%update%') +EXECUTE PROCEDURE trigger_data(23,'skidoo'); +CREATE TRIGGER trig_row_after_insupd +AFTER INSERT OR UPDATE ON rem1 +FOR EACH ROW +WHEN (NEW.f2 like '%update%') +EXECUTE PROCEDURE trigger_data(23,'skidoo'); +-- Insert or update not matching: nothing happens +INSERT INTO rem1 values(1, 'insert'); +UPDATE rem1 set f2 = 'test'; +-- Insert or update matching: triggers are fired +INSERT INTO rem1 values(2, 'update'); +NOTICE: trig_row_before_insupd(23, skidoo) BEFORE ROW INSERT ON rem1 +NOTICE: NEW: (2,update) +NOTICE: trig_row_after_insupd(23, skidoo) AFTER ROW INSERT ON rem1 +NOTICE: NEW: (2,update) +UPDATE rem1 set f2 = 'update update' where f1 = '2'; +NOTICE: trig_row_before_insupd(23, skidoo) BEFORE ROW UPDATE ON rem1 +NOTICE: OLD: (2,update),NEW: (2,"update update") +NOTICE: trig_row_after_insupd(23, skidoo) AFTER ROW UPDATE ON rem1 +NOTICE: OLD: (2,update),NEW: (2,"update update") +CREATE TRIGGER trig_row_before_delete +BEFORE DELETE ON rem1 +FOR EACH ROW +WHEN (OLD.f2 like '%update%') +EXECUTE PROCEDURE trigger_data(23,'skidoo'); +CREATE TRIGGER trig_row_after_delete +AFTER DELETE ON rem1 +FOR EACH ROW +WHEN (OLD.f2 like '%update%') +EXECUTE PROCEDURE trigger_data(23,'skidoo'); +-- Trigger is fired for f1=2, not for f1=1 +DELETE FROM rem1; +NOTICE: trig_row_before_delete(23, skidoo) BEFORE ROW DELETE ON rem1 +NOTICE: OLD: (2,"update update") +NOTICE: trig_row_after_delete(23, skidoo) AFTER ROW DELETE ON rem1 +NOTICE: OLD: (2,"update update") +-- cleanup +DROP TRIGGER trig_row_before_insupd ON rem1; +DROP TRIGGER trig_row_after_insupd ON rem1; +DROP TRIGGER trig_row_before_delete ON rem1; +DROP TRIGGER trig_row_after_delete ON rem1; +-- Test various RETURN statements in BEFORE triggers. +CREATE FUNCTION trig_row_before_insupdate() RETURNS TRIGGER AS $$ + BEGIN + NEW.f2 := NEW.f2 || ' triggered !'; + RETURN NEW; + END +$$ language plpgsql; +CREATE TRIGGER trig_row_before_insupd +BEFORE INSERT OR UPDATE ON rem1 +FOR EACH ROW EXECUTE PROCEDURE trig_row_before_insupdate(); +-- The new values should have 'triggered' appended +INSERT INTO rem1 values(1, 'insert'); +SELECT * from loc1; + f1 | f2 +----+-------------------- + 1 | insert triggered ! +(1 row) + +INSERT INTO rem1 values(2, 'insert') RETURNING f2; + f2 +-------------------- + insert triggered ! +(1 row) + +SELECT * from loc1; + f1 | f2 +----+-------------------- + 1 | insert triggered ! + 2 | insert triggered ! +(2 rows) + +UPDATE rem1 set f2 = ''; +SELECT * from loc1; + f1 | f2 +----+-------------- + 1 | triggered ! + 2 | triggered ! +(2 rows) + +UPDATE rem1 set f2 = 'skidoo' RETURNING f2; + f2 +-------------------- + skidoo triggered ! + skidoo triggered ! +(2 rows) + +SELECT * from loc1; + f1 | f2 +----+-------------------- + 1 | skidoo triggered ! + 2 | skidoo triggered ! +(2 rows) + +DELETE FROM rem1; +-- Add a second trigger, to check that the changes are propagated correctly +-- from trigger to trigger +CREATE TRIGGER trig_row_before_insupd2 +BEFORE INSERT OR UPDATE ON rem1 +FOR EACH ROW EXECUTE PROCEDURE trig_row_before_insupdate(); +INSERT INTO rem1 values(1, 'insert'); +SELECT * from loc1; + f1 | f2 +----+-------------------------------- + 1 | insert triggered ! triggered ! +(1 row) + +INSERT INTO rem1 values(2, 'insert') RETURNING f2; + f2 +-------------------------------- + insert triggered ! triggered ! +(1 row) + +SELECT * from loc1; + f1 | f2 +----+-------------------------------- + 1 | insert triggered ! triggered ! + 2 | insert triggered ! triggered ! +(2 rows) + +UPDATE rem1 set f2 = ''; +SELECT * from loc1; + f1 | f2 +----+-------------------------- + 1 | triggered ! triggered ! + 2 | triggered ! triggered ! +(2 rows) + +UPDATE rem1 set f2 = 'skidoo' RETURNING f2; + f2 +-------------------------------- + skidoo triggered ! triggered ! + skidoo triggered ! triggered ! +(2 rows) + +SELECT * from loc1; + f1 | f2 +----+-------------------------------- + 1 | skidoo triggered ! triggered ! + 2 | skidoo triggered ! triggered ! +(2 rows) + +DROP TRIGGER trig_row_before_insupd ON rem1; +DROP TRIGGER trig_row_before_insupd2 ON rem1; +DELETE from rem1; +INSERT INTO rem1 VALUES (1, 'test'); +-- Test with a trigger returning NULL +CREATE FUNCTION trig_null() RETURNS TRIGGER AS $$ + BEGIN + RETURN NULL; + END +$$ language plpgsql; +CREATE TRIGGER trig_null +BEFORE INSERT OR UPDATE OR DELETE ON rem1 +FOR EACH ROW EXECUTE PROCEDURE trig_null(); +-- Nothing should have changed. +INSERT INTO rem1 VALUES (2, 'test2'); +SELECT * from loc1; + f1 | f2 +----+------ + 1 | test +(1 row) + +UPDATE rem1 SET f2 = 'test2'; +SELECT * from loc1; + f1 | f2 +----+------ + 1 | test +(1 row) + +DELETE from rem1; +SELECT * from loc1; + f1 | f2 +----+------ + 1 | test +(1 row) + +DROP TRIGGER trig_null ON rem1; +DELETE from rem1; +-- Test a combination of local and remote triggers +CREATE TRIGGER trig_row_before +BEFORE INSERT OR UPDATE OR DELETE ON rem1 +FOR EACH ROW EXECUTE PROCEDURE trigger_data(23,'skidoo'); +CREATE TRIGGER trig_row_after +AFTER INSERT OR UPDATE OR DELETE ON rem1 +FOR EACH ROW EXECUTE PROCEDURE trigger_data(23,'skidoo'); +CREATE TRIGGER trig_local_before BEFORE INSERT OR UPDATE ON loc1 +FOR EACH ROW EXECUTE PROCEDURE trig_row_before_insupdate(); +INSERT INTO rem1(f2) VALUES ('test'); +NOTICE: trig_row_before(23, skidoo) BEFORE ROW INSERT ON rem1 +NOTICE: NEW: (12,test) +NOTICE: trig_row_after(23, skidoo) AFTER ROW INSERT ON rem1 +NOTICE: NEW: (12,"test triggered !") +UPDATE rem1 SET f2 = 'testo'; +NOTICE: trig_row_before(23, skidoo) BEFORE ROW UPDATE ON rem1 +NOTICE: OLD: (12,"test triggered !"),NEW: (12,testo) +NOTICE: trig_row_after(23, skidoo) AFTER ROW UPDATE ON rem1 +NOTICE: OLD: (12,"test triggered !"),NEW: (12,"testo triggered !") +-- Test returning system attributes +INSERT INTO rem1(f2) VALUES ('test') RETURNING ctid, xmin, xmax; +NOTICE: trig_row_before(23, skidoo) BEFORE ROW INSERT ON rem1 +NOTICE: NEW: (13,test) +NOTICE: trig_row_after(23, skidoo) AFTER ROW INSERT ON rem1 +NOTICE: NEW: (13,"test triggered !") + ctid | xmin | xmax +--------+------+------------ + (0,27) | 180 | 4294967295 +(1 row) + diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index 1ae1c184372..d7c5fa21195 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -108,7 +108,7 @@ enum FdwScanPrivateIndex * 1) INSERT/UPDATE/DELETE statement text to be sent to the remote server * 2) Integer list of target attribute numbers for INSERT/UPDATE * (NIL for a DELETE) - * 3) Boolean flag showing if there's a RETURNING clause + * 3) Boolean flag showing if the remote query has a RETURNING clause * 4) Integer list of attribute numbers retrieved by RETURNING, if any */ enum FdwModifyPrivateIndex @@ -1246,7 +1246,7 @@ postgresPlanForeignModify(PlannerInfo *root, */ return list_make4(makeString(sql.data), targetAttrs, - makeInteger((returningList != NIL)), + makeInteger((retrieved_attrs != NIL)), retrieved_attrs); } diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 21b15ca9ff2..d47ceca3c11 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -390,3 +390,219 @@ insert into loc1(f2) values('bye'); insert into rem1(f2) values('bye remote'); select * from loc1; select * from rem1; + +-- =================================================================== +-- test local triggers +-- =================================================================== + +-- Trigger functions "borrowed" from triggers regress test. +CREATE FUNCTION trigger_func() RETURNS trigger LANGUAGE plpgsql AS $$ +BEGIN + RAISE NOTICE 'trigger_func(%) called: action = %, when = %, level = %', + TG_ARGV[0], TG_OP, TG_WHEN, TG_LEVEL; + RETURN NULL; +END;$$; + +CREATE TRIGGER trig_stmt_before BEFORE DELETE OR INSERT OR UPDATE ON rem1 + FOR EACH STATEMENT EXECUTE PROCEDURE trigger_func(); +CREATE TRIGGER trig_stmt_after AFTER DELETE OR INSERT OR UPDATE ON rem1 + FOR EACH STATEMENT EXECUTE PROCEDURE trigger_func(); + +CREATE OR REPLACE FUNCTION trigger_data() RETURNS trigger +LANGUAGE plpgsql AS $$ + +declare + oldnew text[]; + relid text; + argstr text; +begin + + relid := TG_relid::regclass; + argstr := ''; + for i in 0 .. TG_nargs - 1 loop + if i > 0 then + argstr := argstr || ', '; + end if; + argstr := argstr || TG_argv[i]; + end loop; + + RAISE NOTICE '%(%) % % % ON %', + tg_name, argstr, TG_when, TG_level, TG_OP, relid; + oldnew := '{}'::text[]; + if TG_OP != 'INSERT' then + oldnew := array_append(oldnew, format('OLD: %s', OLD)); + end if; + + if TG_OP != 'DELETE' then + oldnew := array_append(oldnew, format('NEW: %s', NEW)); + end if; + + RAISE NOTICE '%', array_to_string(oldnew, ','); + + if TG_OP = 'DELETE' then + return OLD; + else + return NEW; + end if; +end; +$$; + +-- Test basic functionality +CREATE TRIGGER trig_row_before +BEFORE INSERT OR UPDATE OR DELETE ON rem1 +FOR EACH ROW EXECUTE PROCEDURE trigger_data(23,'skidoo'); + +CREATE TRIGGER trig_row_after +AFTER INSERT OR UPDATE OR DELETE ON rem1 +FOR EACH ROW EXECUTE PROCEDURE trigger_data(23,'skidoo'); + +delete from rem1; +insert into rem1 values(1,'insert'); +update rem1 set f2 = 'update' where f1 = 1; +update rem1 set f2 = f2 || f2; + + +-- cleanup +DROP TRIGGER trig_row_before ON rem1; +DROP TRIGGER trig_row_after ON rem1; +DROP TRIGGER trig_stmt_before ON rem1; +DROP TRIGGER trig_stmt_after ON rem1; + +DELETE from rem1; + + +-- Test WHEN conditions + +CREATE TRIGGER trig_row_before_insupd +BEFORE INSERT OR UPDATE ON rem1 +FOR EACH ROW +WHEN (NEW.f2 like '%update%') +EXECUTE PROCEDURE trigger_data(23,'skidoo'); + +CREATE TRIGGER trig_row_after_insupd +AFTER INSERT OR UPDATE ON rem1 +FOR EACH ROW +WHEN (NEW.f2 like '%update%') +EXECUTE PROCEDURE trigger_data(23,'skidoo'); + +-- Insert or update not matching: nothing happens +INSERT INTO rem1 values(1, 'insert'); +UPDATE rem1 set f2 = 'test'; + +-- Insert or update matching: triggers are fired +INSERT INTO rem1 values(2, 'update'); +UPDATE rem1 set f2 = 'update update' where f1 = '2'; + +CREATE TRIGGER trig_row_before_delete +BEFORE DELETE ON rem1 +FOR EACH ROW +WHEN (OLD.f2 like '%update%') +EXECUTE PROCEDURE trigger_data(23,'skidoo'); + +CREATE TRIGGER trig_row_after_delete +AFTER DELETE ON rem1 +FOR EACH ROW +WHEN (OLD.f2 like '%update%') +EXECUTE PROCEDURE trigger_data(23,'skidoo'); + +-- Trigger is fired for f1=2, not for f1=1 +DELETE FROM rem1; + +-- cleanup +DROP TRIGGER trig_row_before_insupd ON rem1; +DROP TRIGGER trig_row_after_insupd ON rem1; +DROP TRIGGER trig_row_before_delete ON rem1; +DROP TRIGGER trig_row_after_delete ON rem1; + + +-- Test various RETURN statements in BEFORE triggers. + +CREATE FUNCTION trig_row_before_insupdate() RETURNS TRIGGER AS $$ + BEGIN + NEW.f2 := NEW.f2 || ' triggered !'; + RETURN NEW; + END +$$ language plpgsql; + +CREATE TRIGGER trig_row_before_insupd +BEFORE INSERT OR UPDATE ON rem1 +FOR EACH ROW EXECUTE PROCEDURE trig_row_before_insupdate(); + +-- The new values should have 'triggered' appended +INSERT INTO rem1 values(1, 'insert'); +SELECT * from loc1; +INSERT INTO rem1 values(2, 'insert') RETURNING f2; +SELECT * from loc1; +UPDATE rem1 set f2 = ''; +SELECT * from loc1; +UPDATE rem1 set f2 = 'skidoo' RETURNING f2; +SELECT * from loc1; + +DELETE FROM rem1; + +-- Add a second trigger, to check that the changes are propagated correctly +-- from trigger to trigger +CREATE TRIGGER trig_row_before_insupd2 +BEFORE INSERT OR UPDATE ON rem1 +FOR EACH ROW EXECUTE PROCEDURE trig_row_before_insupdate(); + +INSERT INTO rem1 values(1, 'insert'); +SELECT * from loc1; +INSERT INTO rem1 values(2, 'insert') RETURNING f2; +SELECT * from loc1; +UPDATE rem1 set f2 = ''; +SELECT * from loc1; +UPDATE rem1 set f2 = 'skidoo' RETURNING f2; +SELECT * from loc1; + +DROP TRIGGER trig_row_before_insupd ON rem1; +DROP TRIGGER trig_row_before_insupd2 ON rem1; + +DELETE from rem1; + +INSERT INTO rem1 VALUES (1, 'test'); + +-- Test with a trigger returning NULL +CREATE FUNCTION trig_null() RETURNS TRIGGER AS $$ + BEGIN + RETURN NULL; + END +$$ language plpgsql; + +CREATE TRIGGER trig_null +BEFORE INSERT OR UPDATE OR DELETE ON rem1 +FOR EACH ROW EXECUTE PROCEDURE trig_null(); + +-- Nothing should have changed. +INSERT INTO rem1 VALUES (2, 'test2'); + +SELECT * from loc1; + +UPDATE rem1 SET f2 = 'test2'; + +SELECT * from loc1; + +DELETE from rem1; + +SELECT * from loc1; + +DROP TRIGGER trig_null ON rem1; +DELETE from rem1; + +-- Test a combination of local and remote triggers +CREATE TRIGGER trig_row_before +BEFORE INSERT OR UPDATE OR DELETE ON rem1 +FOR EACH ROW EXECUTE PROCEDURE trigger_data(23,'skidoo'); + +CREATE TRIGGER trig_row_after +AFTER INSERT OR UPDATE OR DELETE ON rem1 +FOR EACH ROW EXECUTE PROCEDURE trigger_data(23,'skidoo'); + +CREATE TRIGGER trig_local_before BEFORE INSERT OR UPDATE ON loc1 +FOR EACH ROW EXECUTE PROCEDURE trig_row_before_insupdate(); + +INSERT INTO rem1(f2) VALUES ('test'); +UPDATE rem1 SET f2 = 'testo'; + +-- Test returning system attributes +INSERT INTO rem1(f2) VALUES ('test') RETURNING ctid, xmin, xmax; diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml index 6c06f1a4367..9c818cd5943 100644 --- a/doc/src/sgml/fdwhandler.sgml +++ b/doc/src/sgml/fdwhandler.sgml @@ -308,7 +308,8 @@ AddForeignUpdateTargets (Query *parsetree, extra values to be fetched. Each such entry must be marked <structfield>resjunk</> = <literal>true</>, and must have a distinct <structfield>resname</> that will identify it at execution time. - Avoid using names matching <literal>ctid<replaceable>N</></literal> or + Avoid using names matching <literal>ctid<replaceable>N</></literal>, + <literal>wholerow</literal>, or <literal>wholerow<replaceable>N</></literal>, as the core system can generate junk columns of these names. </para> @@ -447,11 +448,12 @@ ExecForeignInsert (EState *estate, <para> The data in the returned slot is used only if the <command>INSERT</> - query has a <literal>RETURNING</> clause. Hence, the FDW could choose - to optimize away returning some or all columns depending on the contents - of the <literal>RETURNING</> clause. However, some slot must be - returned to indicate success, or the query's reported row count will be - wrong. + query has a <literal>RETURNING</> clause or the foreign table has + an <literal>AFTER ROW</> trigger. Triggers require all columns, but the + FDW could choose to optimize away returning some or all columns depending + on the contents of the <literal>RETURNING</> clause. Regardless, some + slot must be returned to indicate success, or the query's reported row + count will be wrong. </para> <para> @@ -492,11 +494,12 @@ ExecForeignUpdate (EState *estate, <para> The data in the returned slot is used only if the <command>UPDATE</> - query has a <literal>RETURNING</> clause. Hence, the FDW could choose - to optimize away returning some or all columns depending on the contents - of the <literal>RETURNING</> clause. However, some slot must be - returned to indicate success, or the query's reported row count will be - wrong. + query has a <literal>RETURNING</> clause or the foreign table has + an <literal>AFTER ROW</> trigger. Triggers require all columns, but the + FDW could choose to optimize away returning some or all columns depending + on the contents of the <literal>RETURNING</> clause. Regardless, some + slot must be returned to indicate success, or the query's reported row + count will be wrong. </para> <para> @@ -535,11 +538,12 @@ ExecForeignDelete (EState *estate, <para> The data in the returned slot is used only if the <command>DELETE</> - query has a <literal>RETURNING</> clause. Hence, the FDW could choose - to optimize away returning some or all columns depending on the contents - of the <literal>RETURNING</> clause. However, some slot must be - returned to indicate success, or the query's reported row count will be - wrong. + query has a <literal>RETURNING</> clause or the foreign table has + an <literal>AFTER ROW</> trigger. Triggers require all columns, but the + FDW could choose to optimize away returning some or all columns depending + on the contents of the <literal>RETURNING</> clause. Regardless, some + slot must be returned to indicate success, or the query's reported row + count will be wrong. </para> <para> diff --git a/doc/src/sgml/ref/create_trigger.sgml b/doc/src/sgml/ref/create_trigger.sgml index a8fba49e4c3..d270d66c574 100644 --- a/doc/src/sgml/ref/create_trigger.sgml +++ b/doc/src/sgml/ref/create_trigger.sgml @@ -43,9 +43,10 @@ CREATE [ CONSTRAINT ] TRIGGER <replaceable class="PARAMETER">name</replaceable> <para> <command>CREATE TRIGGER</command> creates a new trigger. The - trigger will be associated with the specified table or view and will - execute the specified function <replaceable - class="parameter">function_name</replaceable> when certain events occur. + trigger will be associated with the specified table, view, or foreign table + and will execute the specified + function <replaceable class="parameter">function_name</replaceable> when + certain events occur. </para> <para> @@ -93,7 +94,7 @@ CREATE [ CONSTRAINT ] TRIGGER <replaceable class="PARAMETER">name</replaceable> <para> The following table summarizes which types of triggers may be used on - tables and views: + tables, views, and foreign tables: </para> <informaltable id="supported-trigger-types"> @@ -110,8 +111,8 @@ CREATE [ CONSTRAINT ] TRIGGER <replaceable class="PARAMETER">name</replaceable> <row> <entry align="center" morerows="1"><literal>BEFORE</></entry> <entry align="center"><command>INSERT</>/<command>UPDATE</>/<command>DELETE</></entry> - <entry align="center">Tables</entry> - <entry align="center">Tables and views</entry> + <entry align="center">Tables and foreign tables</entry> + <entry align="center">Tables, views, and foreign tables</entry> </row> <row> <entry align="center"><command>TRUNCATE</></entry> @@ -121,8 +122,8 @@ CREATE [ CONSTRAINT ] TRIGGER <replaceable class="PARAMETER">name</replaceable> <row> <entry align="center" morerows="1"><literal>AFTER</></entry> <entry align="center"><command>INSERT</>/<command>UPDATE</>/<command>DELETE</></entry> - <entry align="center">Tables</entry> - <entry align="center">Tables and views</entry> + <entry align="center">Tables and foreign tables</entry> + <entry align="center">Tables, views, and foreign tables</entry> </row> <row> <entry align="center"><command>TRUNCATE</></entry> @@ -164,13 +165,13 @@ CREATE [ CONSTRAINT ] TRIGGER <replaceable class="PARAMETER">name</replaceable> <firstterm>constraint trigger</>. This is the same as a regular trigger except that the timing of the trigger firing can be adjusted using <xref linkend="SQL-SET-CONSTRAINTS">. - Constraint triggers must be <literal>AFTER ROW</> triggers. They can - be fired either at the end of the statement causing the triggering event, - or at the end of the containing transaction; in the latter case they are - said to be <firstterm>deferred</>. A pending deferred-trigger firing can - also be forced to happen immediately by using <command>SET CONSTRAINTS</>. - Constraint triggers are expected to raise an exception when the constraints - they implement are violated. + Constraint triggers must be <literal>AFTER ROW</> triggers on tables. They + can be fired either at the end of the statement causing the triggering + event, or at the end of the containing transaction; in the latter case they + are said to be <firstterm>deferred</>. A pending deferred-trigger firing + can also be forced to happen immediately by using <command>SET + CONSTRAINTS</>. Constraint triggers are expected to raise an exception + when the constraints they implement are violated. </para> <para> @@ -244,8 +245,8 @@ UPDATE OF <replaceable>column_name1</replaceable> [, <replaceable>column_name2</ <term><replaceable class="parameter">table_name</replaceable></term> <listitem> <para> - The name (optionally schema-qualified) of the table or view the trigger - is for. + The name (optionally schema-qualified) of the table, view, or foreign + table the trigger is for. </para> </listitem> </varlistentry> @@ -481,6 +482,14 @@ CREATE TRIGGER view_insert <refsect1 id="SQL-CREATETRIGGER-compatibility"> <title>Compatibility</title> + <!-- + It's not clear whether SQL/MED contemplates triggers on foreign tables. + Its <drop basic column definition> General Rules do mention the possibility + of a reference from a trigger column list. On the other hand, nothing + overrides the fact that CREATE TRIGGER only targets base tables. For now, + do not document the compatibility status of triggers on foreign tables. + --> + <para> The <command>CREATE TRIGGER</command> statement in <productname>PostgreSQL</productname> implements a subset of the diff --git a/doc/src/sgml/trigger.sgml b/doc/src/sgml/trigger.sgml index f579340e48f..f94aea174ab 100644 --- a/doc/src/sgml/trigger.sgml +++ b/doc/src/sgml/trigger.sgml @@ -33,20 +33,21 @@ <para> A trigger is a specification that the database should automatically execute a particular function whenever a certain type of operation is - performed. Triggers can be attached to both tables and views. + performed. Triggers can be attached to tables, views, and foreign tables. </para> <para> - On tables, triggers can be defined to execute either before or after any - <command>INSERT</command>, <command>UPDATE</command>, or - <command>DELETE</command> operation, either once per modified row, + On tables and foreign tables, triggers can be defined to execute either + before or after any <command>INSERT</command>, <command>UPDATE</command>, + or <command>DELETE</command> operation, either once per modified row, or once per <acronym>SQL</acronym> statement. <command>UPDATE</command> triggers can moreover be set to fire only if certain columns are mentioned in the <literal>SET</literal> clause of the <command>UPDATE</command> statement. Triggers can also fire for <command>TRUNCATE</command> statements. If a trigger event occurs, the trigger's function is called at the - appropriate time to handle the event. + appropriate time to handle the event. Foreign tables do not support the + TRUNCATE statement at all. </para> <para> @@ -111,10 +112,10 @@ triggers fire immediately before a particular row is operated on, while row-level <literal>AFTER</> triggers fire at the end of the statement (but before any statement-level <literal>AFTER</> triggers). - These types of triggers may only be defined on tables. Row-level - <literal>INSTEAD OF</> triggers may only be defined on views, and fire - immediately as each row in the view is identified as needing to be - operated on. + These types of triggers may only be defined on tables and foreign tables. + Row-level <literal>INSTEAD OF</> triggers may only be defined on views, + and fire immediately as each row in the view is identified as needing to + be operated on. </para> <para> @@ -548,7 +549,8 @@ typedef struct TriggerData <command>DELETE</command> then this is what you should return from the function if you don't want to replace the row with a different one (in the case of <command>INSERT</command>) or - skip the operation. + skip the operation. For triggers on foreign tables, values of system + columns herein are unspecified. </para> </listitem> </varlistentry> @@ -563,7 +565,8 @@ typedef struct TriggerData <command>DELETE</command>. This is what you have to return from the function if the event is an <command>UPDATE</command> and you don't want to replace this row by a different one or - skip the operation. + skip the operation. For triggers on foreign tables, values of system + columns herein are unspecified. </para> </listitem> </varlistentry> diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 25f01e5165f..7f3f730a87b 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -3180,6 +3180,9 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, case AT_DisableTrig: /* DISABLE TRIGGER variants */ case AT_DisableTrigAll: case AT_DisableTrigUser: + ATSimplePermissions(rel, ATT_TABLE | ATT_FOREIGN_TABLE); + pass = AT_PASS_MISC; + break; case AT_EnableRule: /* ENABLE/DISABLE RULE variants */ case AT_EnableAlwaysRule: case AT_EnableReplicaRule: diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 3e92a7c29e5..5f1ccf02c27 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -56,6 +56,7 @@ #include "utils/snapmgr.h" #include "utils/syscache.h" #include "utils/tqual.h" +#include "utils/tuplestore.h" /* GUC variables */ @@ -195,6 +196,30 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, RelationGetRelationName(rel)), errdetail("Views cannot have TRUNCATE triggers."))); } + else if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE) + { + if (stmt->timing != TRIGGER_TYPE_BEFORE && + stmt->timing != TRIGGER_TYPE_AFTER) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("\"%s\" is a foreign table", + RelationGetRelationName(rel)), + errdetail("Foreign tables cannot have INSTEAD OF triggers."))); + + if (TRIGGER_FOR_TRUNCATE(stmt->events)) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("\"%s\" is a foreign table", + RelationGetRelationName(rel)), + errdetail("Foreign tables cannot have TRUNCATE triggers."))); + + if (stmt->isconstraint) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("\"%s\" is a foreign table", + RelationGetRelationName(rel)), + errdetail("Foreign tables cannot have constraint triggers."))); + } else ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), @@ -1080,10 +1105,11 @@ RemoveTriggerById(Oid trigOid) rel = heap_open(relid, AccessExclusiveLock); if (rel->rd_rel->relkind != RELKIND_RELATION && - rel->rd_rel->relkind != RELKIND_VIEW) + rel->rd_rel->relkind != RELKIND_VIEW && + rel->rd_rel->relkind != RELKIND_FOREIGN_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table or view", + errmsg("\"%s\" is not a table, view, or foreign table", RelationGetRelationName(rel)))); if (!allowSystemTableMods && IsSystemRelation(rel)) @@ -1184,10 +1210,12 @@ RangeVarCallbackForRenameTrigger(const RangeVar *rv, Oid relid, Oid oldrelid, form = (Form_pg_class) GETSTRUCT(tuple); /* only tables and views can have triggers */ - if (form->relkind != RELKIND_RELATION && form->relkind != RELKIND_VIEW) + if (form->relkind != RELKIND_RELATION && form->relkind != RELKIND_VIEW && + form->relkind != RELKIND_FOREIGN_TABLE) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("\"%s\" is not a table or view", rv->relname))); + errmsg("\"%s\" is not a table, view, or foreign table", + rv->relname))); /* you must own the table to rename one of its triggers */ if (!pg_class_ownercheck(relid, GetUserId())) @@ -2164,7 +2192,8 @@ ExecASDeleteTriggers(EState *estate, ResultRelInfo *relinfo) bool ExecBRDeleteTriggers(EState *estate, EPQState *epqstate, ResultRelInfo *relinfo, - ItemPointer tupleid) + ItemPointer tupleid, + HeapTuple fdw_trigtuple) { TriggerDesc *trigdesc = relinfo->ri_TrigDesc; bool result = true; @@ -2174,10 +2203,16 @@ ExecBRDeleteTriggers(EState *estate, EPQState *epqstate, TupleTableSlot *newSlot; int i; - trigtuple = GetTupleForTrigger(estate, epqstate, relinfo, tupleid, - LockTupleExclusive, &newSlot); - if (trigtuple == NULL) - return false; + Assert(HeapTupleIsValid(fdw_trigtuple) ^ ItemPointerIsValid(tupleid)); + if (fdw_trigtuple == NULL) + { + trigtuple = GetTupleForTrigger(estate, epqstate, relinfo, tupleid, + LockTupleExclusive, &newSlot); + if (trigtuple == NULL) + return false; + } + else + trigtuple = fdw_trigtuple; LocTriggerData.type = T_TriggerData; LocTriggerData.tg_event = TRIGGER_EVENT_DELETE | @@ -2215,29 +2250,38 @@ ExecBRDeleteTriggers(EState *estate, EPQState *epqstate, if (newtuple != trigtuple) heap_freetuple(newtuple); } - heap_freetuple(trigtuple); + if (trigtuple != fdw_trigtuple) + heap_freetuple(trigtuple); return result; } void ExecARDeleteTriggers(EState *estate, ResultRelInfo *relinfo, - ItemPointer tupleid) + ItemPointer tupleid, + HeapTuple fdw_trigtuple) { TriggerDesc *trigdesc = relinfo->ri_TrigDesc; if (trigdesc && trigdesc->trig_delete_after_row) { - HeapTuple trigtuple = GetTupleForTrigger(estate, - NULL, - relinfo, - tupleid, - LockTupleExclusive, - NULL); + HeapTuple trigtuple; + + Assert(HeapTupleIsValid(fdw_trigtuple) ^ ItemPointerIsValid(tupleid)); + if (fdw_trigtuple == NULL) + trigtuple = GetTupleForTrigger(estate, + NULL, + relinfo, + tupleid, + LockTupleExclusive, + NULL); + else + trigtuple = fdw_trigtuple; AfterTriggerSaveEvent(estate, relinfo, TRIGGER_EVENT_DELETE, true, trigtuple, NULL, NIL, NULL); - heap_freetuple(trigtuple); + if (trigtuple != fdw_trigtuple) + heap_freetuple(trigtuple); } } @@ -2353,7 +2397,9 @@ ExecASUpdateTriggers(EState *estate, ResultRelInfo *relinfo) TupleTableSlot * ExecBRUpdateTriggers(EState *estate, EPQState *epqstate, ResultRelInfo *relinfo, - ItemPointer tupleid, TupleTableSlot *slot) + ItemPointer tupleid, + HeapTuple fdw_trigtuple, + TupleTableSlot *slot) { TriggerDesc *trigdesc = relinfo->ri_TrigDesc; HeapTuple slottuple = ExecMaterializeSlot(slot); @@ -2380,11 +2426,20 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate, else lockmode = LockTupleNoKeyExclusive; - /* get a copy of the on-disk tuple we are planning to update */ - trigtuple = GetTupleForTrigger(estate, epqstate, relinfo, tupleid, - lockmode, &newSlot); - if (trigtuple == NULL) - return NULL; /* cancel the update action */ + Assert(HeapTupleIsValid(fdw_trigtuple) ^ ItemPointerIsValid(tupleid)); + if (fdw_trigtuple == NULL) + { + /* get a copy of the on-disk tuple we are planning to update */ + trigtuple = GetTupleForTrigger(estate, epqstate, relinfo, tupleid, + lockmode, &newSlot); + if (trigtuple == NULL) + return NULL; /* cancel the update action */ + } + else + { + trigtuple = fdw_trigtuple; + newSlot = NULL; + } /* * In READ COMMITTED isolation level it's possible that target tuple was @@ -2437,11 +2492,13 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate, heap_freetuple(oldtuple); if (newtuple == NULL) { - heap_freetuple(trigtuple); + if (trigtuple != fdw_trigtuple) + heap_freetuple(trigtuple); return NULL; /* "do nothing" */ } } - heap_freetuple(trigtuple); + if (trigtuple != fdw_trigtuple) + heap_freetuple(trigtuple); if (newtuple != slottuple) { @@ -2464,24 +2521,33 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate, void ExecARUpdateTriggers(EState *estate, ResultRelInfo *relinfo, - ItemPointer tupleid, HeapTuple newtuple, + ItemPointer tupleid, + HeapTuple fdw_trigtuple, + HeapTuple newtuple, List *recheckIndexes) { TriggerDesc *trigdesc = relinfo->ri_TrigDesc; if (trigdesc && trigdesc->trig_update_after_row) { - HeapTuple trigtuple = GetTupleForTrigger(estate, - NULL, - relinfo, - tupleid, - LockTupleExclusive, - NULL); + HeapTuple trigtuple; + + Assert(HeapTupleIsValid(fdw_trigtuple) ^ ItemPointerIsValid(tupleid)); + if (fdw_trigtuple == NULL) + trigtuple = GetTupleForTrigger(estate, + NULL, + relinfo, + tupleid, + LockTupleExclusive, + NULL); + else + trigtuple = fdw_trigtuple; AfterTriggerSaveEvent(estate, relinfo, TRIGGER_EVENT_UPDATE, true, trigtuple, newtuple, recheckIndexes, GetModifiedColumns(relinfo, estate)); - heap_freetuple(trigtuple); + if (trigtuple != fdw_trigtuple) + heap_freetuple(trigtuple); } } @@ -2942,13 +3008,22 @@ typedef SetConstraintStateData *SetConstraintState; * Per-trigger-event data * * The actual per-event data, AfterTriggerEventData, includes DONE/IN_PROGRESS - * status bits and one or two tuple CTIDs. Each event record also has an - * associated AfterTriggerSharedData that is shared across all instances - * of similar events within a "chunk". + * status bits and up to two tuple CTIDs. Each event record also has an + * associated AfterTriggerSharedData that is shared across all instances of + * similar events within a "chunk". * - * We arrange not to waste storage on ate_ctid2 for non-update events. - * We could go further and not store either ctid for statement-level triggers, - * but that seems unlikely to be worth the trouble. + * For row-level triggers, we arrange not to waste storage on unneeded ctid + * fields. Updates of regular tables use two; inserts and deletes of regular + * tables use one; foreign tables always use zero and save the tuple(s) to a + * tuplestore. AFTER_TRIGGER_FDW_FETCH directs AfterTriggerExecute() to + * retrieve a fresh tuple or pair of tuples from that tuplestore, while + * AFTER_TRIGGER_FDW_REUSE directs it to use the most-recently-retrieved + * tuple(s). This permits storing tuples once regardless of the number of + * row-level triggers on a foreign table. + * + * Statement-level triggers always bear AFTER_TRIGGER_1CTID, though they + * require no ctid field. We lack the flag bit space to neatly represent that + * distinct case, and it seems unlikely to be worth much trouble. * * Note: ats_firing_id is initially zero and is set to something else when * AFTER_TRIGGER_IN_PROGRESS is set. It indicates which trigger firing @@ -2963,9 +3038,14 @@ typedef uint32 TriggerFlags; #define AFTER_TRIGGER_OFFSET 0x0FFFFFFF /* must be low-order * bits */ -#define AFTER_TRIGGER_2CTIDS 0x10000000 -#define AFTER_TRIGGER_DONE 0x20000000 -#define AFTER_TRIGGER_IN_PROGRESS 0x40000000 +#define AFTER_TRIGGER_DONE 0x10000000 +#define AFTER_TRIGGER_IN_PROGRESS 0x20000000 +/* bits describing the size and tuple sources of this event */ +#define AFTER_TRIGGER_FDW_REUSE 0x00000000 +#define AFTER_TRIGGER_FDW_FETCH 0x80000000 +#define AFTER_TRIGGER_1CTID 0x40000000 +#define AFTER_TRIGGER_2CTID 0xC0000000 +#define AFTER_TRIGGER_TUP_BITS 0xC0000000 typedef struct AfterTriggerSharedData *AfterTriggerShared; @@ -2986,16 +3066,25 @@ typedef struct AfterTriggerEventData ItemPointerData ate_ctid2; /* new updated tuple */ } AfterTriggerEventData; -/* This struct must exactly match the one above except for not having ctid2 */ +/* AfterTriggerEventData, minus ate_ctid2 */ typedef struct AfterTriggerEventDataOneCtid { TriggerFlags ate_flags; /* status bits and offset to shared data */ ItemPointerData ate_ctid1; /* inserted, deleted, or old updated tuple */ } AfterTriggerEventDataOneCtid; +/* AfterTriggerEventData, minus ate_ctid1 and ate_ctid2 */ +typedef struct AfterTriggerEventDataZeroCtids +{ + TriggerFlags ate_flags; /* status bits and offset to shared data */ +} AfterTriggerEventDataZeroCtids; + #define SizeofTriggerEvent(evt) \ - (((evt)->ate_flags & AFTER_TRIGGER_2CTIDS) ? \ - sizeof(AfterTriggerEventData) : sizeof(AfterTriggerEventDataOneCtid)) + (((evt)->ate_flags & AFTER_TRIGGER_TUP_BITS) == AFTER_TRIGGER_2CTID ? \ + sizeof(AfterTriggerEventData) : \ + ((evt)->ate_flags & AFTER_TRIGGER_TUP_BITS) == AFTER_TRIGGER_1CTID ? \ + sizeof(AfterTriggerEventDataOneCtid) : \ + sizeof(AfterTriggerEventDataZeroCtids)) #define GetTriggerSharedData(evt) \ ((AfterTriggerShared) ((char *) (evt) + ((evt)->ate_flags & AFTER_TRIGGER_OFFSET))) @@ -3068,7 +3157,11 @@ typedef struct AfterTriggerEventList * immediate-mode triggers, and append any deferred events to the main events * list. * - * maxquerydepth is just the allocated length of query_stack. + * fdw_tuplestores[query_depth] is a tuplestore containing the foreign tuples + * needed for the current query. + * + * maxquerydepth is just the allocated length of query_stack and + * fdw_tuplestores. * * state_stack is a stack of pointers to saved copies of the SET CONSTRAINTS * state data; each subtransaction level that modifies that state first @@ -3097,6 +3190,7 @@ typedef struct AfterTriggersData AfterTriggerEventList events; /* deferred-event list */ int query_depth; /* current query list index */ AfterTriggerEventList *query_stack; /* events pending from each query */ + Tuplestorestate **fdw_tuplestores; /* foreign tuples from each query */ int maxquerydepth; /* allocated len of above array */ MemoryContext event_cxt; /* memory context for events, if any */ @@ -3113,18 +3207,60 @@ typedef AfterTriggersData *AfterTriggers; static AfterTriggers afterTriggers; - static void AfterTriggerExecute(AfterTriggerEvent event, Relation rel, TriggerDesc *trigdesc, FmgrInfo *finfo, Instrumentation *instr, - MemoryContext per_tuple_context); + MemoryContext per_tuple_context, + TupleTableSlot *trig_tuple_slot1, + TupleTableSlot *trig_tuple_slot2); static SetConstraintState SetConstraintStateCreate(int numalloc); static SetConstraintState SetConstraintStateCopy(SetConstraintState state); static SetConstraintState SetConstraintStateAddItem(SetConstraintState state, Oid tgoid, bool tgisdeferred); +/* + * Gets the current query fdw tuplestore and initializes it if necessary + */ +static Tuplestorestate * +GetCurrentFDWTuplestore() +{ + Tuplestorestate *ret; + + ret = afterTriggers->fdw_tuplestores[afterTriggers->query_depth]; + if (ret == NULL) + { + MemoryContext oldcxt; + ResourceOwner saveResourceOwner; + + /* + * Make the tuplestore valid until end of transaction. This is the + * allocation lifespan of the associated events list, but we really + * only need it until AfterTriggerEndQuery(). + */ + oldcxt = MemoryContextSwitchTo(TopTransactionContext); + saveResourceOwner = CurrentResourceOwner; + PG_TRY(); + { + CurrentResourceOwner = TopTransactionResourceOwner; + ret = tuplestore_begin_heap(false, false, work_mem); + } + PG_CATCH(); + { + CurrentResourceOwner = saveResourceOwner; + PG_RE_THROW(); + } + PG_END_TRY(); + CurrentResourceOwner = saveResourceOwner; + MemoryContextSwitchTo(oldcxt); + + afterTriggers->fdw_tuplestores[afterTriggers->query_depth] = ret; + } + + return ret; +} + /* ---------- * afterTriggerCheckState() * @@ -3365,13 +3501,17 @@ afterTriggerRestoreEventList(AfterTriggerEventList *events, * instr: array of EXPLAIN ANALYZE instrumentation nodes (one per trigger), * or NULL if no instrumentation is wanted. * per_tuple_context: memory context to call trigger function in. + * trig_tuple_slot1: scratch slot for tg_trigtuple (foreign tables only) + * trig_tuple_slot2: scratch slot for tg_newtuple (foreign tables only) * ---------- */ static void AfterTriggerExecute(AfterTriggerEvent event, Relation rel, TriggerDesc *trigdesc, FmgrInfo *finfo, Instrumentation *instr, - MemoryContext per_tuple_context) + MemoryContext per_tuple_context, + TupleTableSlot *trig_tuple_slot1, + TupleTableSlot *trig_tuple_slot2) { AfterTriggerShared evtshared = GetTriggerSharedData(event); Oid tgoid = evtshared->ats_tgoid; @@ -3408,34 +3548,76 @@ AfterTriggerExecute(AfterTriggerEvent event, /* * Fetch the required tuple(s). */ - if (ItemPointerIsValid(&(event->ate_ctid1))) + switch (event->ate_flags & AFTER_TRIGGER_TUP_BITS) { - ItemPointerCopy(&(event->ate_ctid1), &(tuple1.t_self)); - if (!heap_fetch(rel, SnapshotAny, &tuple1, &buffer1, false, NULL)) - elog(ERROR, "failed to fetch tuple1 for AFTER trigger"); - LocTriggerData.tg_trigtuple = &tuple1; - LocTriggerData.tg_trigtuplebuf = buffer1; - } - else - { - LocTriggerData.tg_trigtuple = NULL; - LocTriggerData.tg_trigtuplebuf = InvalidBuffer; - } + case AFTER_TRIGGER_FDW_FETCH: + { + Tuplestorestate *fdw_tuplestore = GetCurrentFDWTuplestore(); - /* don't touch ctid2 if not there */ - if ((event->ate_flags & AFTER_TRIGGER_2CTIDS) && - ItemPointerIsValid(&(event->ate_ctid2))) - { - ItemPointerCopy(&(event->ate_ctid2), &(tuple2.t_self)); - if (!heap_fetch(rel, SnapshotAny, &tuple2, &buffer2, false, NULL)) - elog(ERROR, "failed to fetch tuple2 for AFTER trigger"); - LocTriggerData.tg_newtuple = &tuple2; - LocTriggerData.tg_newtuplebuf = buffer2; - } - else - { - LocTriggerData.tg_newtuple = NULL; - LocTriggerData.tg_newtuplebuf = InvalidBuffer; + if (!tuplestore_gettupleslot(fdw_tuplestore, true, false, + trig_tuple_slot1)) + elog(ERROR, "failed to fetch tuple1 for AFTER trigger"); + + if ((evtshared->ats_event & TRIGGER_EVENT_OPMASK) == + TRIGGER_EVENT_UPDATE && + !tuplestore_gettupleslot(fdw_tuplestore, true, false, + trig_tuple_slot2)) + elog(ERROR, "failed to fetch tuple2 for AFTER trigger"); + } + /* fall through */ + case AFTER_TRIGGER_FDW_REUSE: + /* + * Using ExecMaterializeSlot() rather than ExecFetchSlotTuple() + * ensures that tg_trigtuple does not reference tuplestore memory. + * (It is formally possible for the trigger function to queue + * trigger events that add to the same tuplestore, which can push + * other tuples out of memory.) The distinction is academic, + * because we start with a minimal tuple that ExecFetchSlotTuple() + * must materialize anyway. + */ + LocTriggerData.tg_trigtuple = + ExecMaterializeSlot(trig_tuple_slot1); + LocTriggerData.tg_trigtuplebuf = InvalidBuffer; + + LocTriggerData.tg_newtuple = + ((evtshared->ats_event & TRIGGER_EVENT_OPMASK) == + TRIGGER_EVENT_UPDATE) ? + ExecMaterializeSlot(trig_tuple_slot2) : NULL; + LocTriggerData.tg_newtuplebuf = InvalidBuffer; + + break; + + default: + if (ItemPointerIsValid(&(event->ate_ctid1))) + { + ItemPointerCopy(&(event->ate_ctid1), &(tuple1.t_self)); + if (!heap_fetch(rel, SnapshotAny, &tuple1, &buffer1, false, NULL)) + elog(ERROR, "failed to fetch tuple1 for AFTER trigger"); + LocTriggerData.tg_trigtuple = &tuple1; + LocTriggerData.tg_trigtuplebuf = buffer1; + } + else + { + LocTriggerData.tg_trigtuple = NULL; + LocTriggerData.tg_trigtuplebuf = InvalidBuffer; + } + + /* don't touch ctid2 if not there */ + if ((event->ate_flags & AFTER_TRIGGER_TUP_BITS) == + AFTER_TRIGGER_2CTID && + ItemPointerIsValid(&(event->ate_ctid2))) + { + ItemPointerCopy(&(event->ate_ctid2), &(tuple2.t_self)); + if (!heap_fetch(rel, SnapshotAny, &tuple2, &buffer2, false, NULL)) + elog(ERROR, "failed to fetch tuple2 for AFTER trigger"); + LocTriggerData.tg_newtuple = &tuple2; + LocTriggerData.tg_newtuplebuf = buffer2; + } + else + { + LocTriggerData.tg_newtuple = NULL; + LocTriggerData.tg_newtuplebuf = InvalidBuffer; + } } /* @@ -3457,7 +3639,9 @@ AfterTriggerExecute(AfterTriggerEvent event, finfo, NULL, per_tuple_context); - if (rettuple != NULL && rettuple != &tuple1 && rettuple != &tuple2) + if (rettuple != NULL && + rettuple != LocTriggerData.tg_trigtuple && + rettuple != LocTriggerData.tg_newtuple) heap_freetuple(rettuple); /* @@ -3577,6 +3761,8 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events, TriggerDesc *trigdesc = NULL; FmgrInfo *finfo = NULL; Instrumentation *instr = NULL; + TupleTableSlot *slot1 = NULL, + *slot2 = NULL; /* Make a local EState if need be */ if (estate == NULL) @@ -3621,6 +3807,16 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events, trigdesc = rInfo->ri_TrigDesc; finfo = rInfo->ri_TrigFunctions; instr = rInfo->ri_TrigInstrument; + if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE) + { + if (slot1 != NULL) + { + ExecDropSingleTupleTableSlot(slot1); + ExecDropSingleTupleTableSlot(slot2); + } + slot1 = MakeSingleTupleTableSlot(rel->rd_att); + slot2 = MakeSingleTupleTableSlot(rel->rd_att); + } if (trigdesc == NULL) /* should not happen */ elog(ERROR, "relation %u has no triggers", evtshared->ats_relid); @@ -3632,7 +3828,7 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events, * won't try to re-fire it. */ AfterTriggerExecute(event, rel, trigdesc, finfo, instr, - per_tuple_context); + per_tuple_context, slot1, slot2); /* * Mark the event as done. @@ -3663,6 +3859,11 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events, events->tailfree = chunk->freeptr; } } + if (slot1 != NULL) + { + ExecDropSingleTupleTableSlot(slot1); + ExecDropSingleTupleTableSlot(slot2); + } /* Release working resources */ MemoryContextDelete(per_tuple_context); @@ -3712,10 +3913,13 @@ AfterTriggerBeginXact(void) afterTriggers->events.tailfree = NULL; afterTriggers->query_depth = -1; - /* We initialize the query stack to a reasonable size */ + /* We initialize the arrays to a reasonable size */ afterTriggers->query_stack = (AfterTriggerEventList *) MemoryContextAlloc(TopTransactionContext, 8 * sizeof(AfterTriggerEventList)); + afterTriggers->fdw_tuplestores = (Tuplestorestate **) + MemoryContextAllocZero(TopTransactionContext, + 8 * sizeof(Tuplestorestate *)); afterTriggers->maxquerydepth = 8; /* Context for events is created only when needed */ @@ -3756,11 +3960,18 @@ AfterTriggerBeginQuery(void) if (afterTriggers->query_depth >= afterTriggers->maxquerydepth) { /* repalloc will keep the stack in the same context */ - int new_alloc = afterTriggers->maxquerydepth * 2; + int old_alloc = afterTriggers->maxquerydepth; + int new_alloc = old_alloc * 2; afterTriggers->query_stack = (AfterTriggerEventList *) repalloc(afterTriggers->query_stack, new_alloc * sizeof(AfterTriggerEventList)); + afterTriggers->fdw_tuplestores = (Tuplestorestate **) + repalloc(afterTriggers->fdw_tuplestores, + new_alloc * sizeof(Tuplestorestate *)); + /* Clear newly-allocated slots for subsequent lazy initialization. */ + memset(afterTriggers->fdw_tuplestores + old_alloc, + 0, (new_alloc - old_alloc) * sizeof(Tuplestorestate *)); afterTriggers->maxquerydepth = new_alloc; } @@ -3788,6 +3999,7 @@ void AfterTriggerEndQuery(EState *estate) { AfterTriggerEventList *events; + Tuplestorestate *fdw_tuplestore; /* Must be inside a transaction */ Assert(afterTriggers != NULL); @@ -3832,7 +4044,13 @@ AfterTriggerEndQuery(EState *estate) break; } - /* Release query-local storage for events */ + /* Release query-local storage for events, including tuplestore if any */ + fdw_tuplestore = afterTriggers->fdw_tuplestores[afterTriggers->query_depth]; + if (fdw_tuplestore) + { + tuplestore_end(fdw_tuplestore); + afterTriggers->fdw_tuplestores[afterTriggers->query_depth] = NULL; + } afterTriggerFreeEventList(&afterTriggers->query_stack[afterTriggers->query_depth]); afterTriggers->query_depth--; @@ -4056,6 +4274,15 @@ AfterTriggerEndSubXact(bool isCommit) */ while (afterTriggers->query_depth > afterTriggers->depth_stack[my_level]) { + Tuplestorestate *ts; + + ts = afterTriggers->fdw_tuplestores[afterTriggers->query_depth]; + if (ts) + { + tuplestore_end(ts); + afterTriggers->fdw_tuplestores[afterTriggers->query_depth] = NULL; + } + afterTriggerFreeEventList(&afterTriggers->query_stack[afterTriggers->query_depth]); afterTriggers->query_depth--; } @@ -4552,9 +4779,11 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, TriggerDesc *trigdesc = relinfo->ri_TrigDesc; AfterTriggerEventData new_event; AfterTriggerSharedData new_shared; + char relkind = relinfo->ri_RelationDesc->rd_rel->relkind; int tgtype_event; int tgtype_level; int i; + Tuplestorestate *fdw_tuplestore = NULL; /* * Check state. We use normal tests not Asserts because it is possible to @@ -4573,7 +4802,6 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, * validation is important to make sure we don't walk off the edge of our * arrays. */ - new_event.ate_flags = 0; switch (event) { case TRIGGER_EVENT_INSERT: @@ -4618,7 +4846,6 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, Assert(newtup != NULL); ItemPointerCopy(&(oldtup->t_self), &(new_event.ate_ctid1)); ItemPointerCopy(&(newtup->t_self), &(new_event.ate_ctid2)); - new_event.ate_flags |= AFTER_TRIGGER_2CTIDS; } else { @@ -4641,6 +4868,11 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, break; } + if (!(relkind == RELKIND_FOREIGN_TABLE && row_trigger)) + new_event.ate_flags = (row_trigger && event == TRIGGER_EVENT_UPDATE) ? + AFTER_TRIGGER_2CTID : AFTER_TRIGGER_1CTID; + /* else, we'll initialize ate_flags for each trigger */ + tgtype_level = (row_trigger ? TRIGGER_TYPE_ROW : TRIGGER_TYPE_STATEMENT); for (i = 0; i < trigdesc->numtriggers; i++) @@ -4656,6 +4888,18 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, modifiedCols, oldtup, newtup)) continue; + if (relkind == RELKIND_FOREIGN_TABLE && row_trigger) + { + if (fdw_tuplestore == NULL) + { + fdw_tuplestore = GetCurrentFDWTuplestore(); + new_event.ate_flags = AFTER_TRIGGER_FDW_FETCH; + } + else + /* subsequent event for the same tuple */ + new_event.ate_flags = AFTER_TRIGGER_FDW_REUSE; + } + /* * If the trigger is a foreign key enforcement trigger, there are * certain cases where we can skip queueing the event because we can @@ -4717,6 +4961,19 @@ AfterTriggerSaveEvent(EState *estate, ResultRelInfo *relinfo, afterTriggerAddEvent(&afterTriggers->query_stack[afterTriggers->query_depth], &new_event, &new_shared); } + + /* + * Finally, spool any foreign tuple(s). The tuplestore squashes them to + * minimal tuples, so this loses any system columns. The executor lost + * those columns before us, for an unrelated reason, so this is fine. + */ + if (fdw_tuplestore) + { + if (oldtup != NULL) + tuplestore_puttuple(fdw_tuplestore, oldtup); + if (newtup != NULL) + tuplestore_puttuple(fdw_tuplestore, newtup); + } } Datum diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 6f0f47e7ce3..fca7a2581f3 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -309,15 +309,17 @@ ExecInsert(TupleTableSlot *slot, * delete and oldtuple is NULL. When deleting from a view, * oldtuple is passed to the INSTEAD OF triggers and identifies * what to delete, and tupleid is invalid. When deleting from a - * foreign table, both tupleid and oldtuple are NULL; the FDW has - * to figure out which row to delete using data from the planSlot. + * foreign table, tupleid is invalid; the FDW has to figure out + * which row to delete using data from the planSlot. oldtuple is + * passed to foreign table triggers; it is NULL when the foreign + * table has no relevant triggers. * * Returns RETURNING result if any, otherwise NULL. * ---------------------------------------------------------------- */ static TupleTableSlot * ExecDelete(ItemPointer tupleid, - HeapTupleHeader oldtuple, + HeapTuple oldtuple, TupleTableSlot *planSlot, EPQState *epqstate, EState *estate, @@ -342,7 +344,7 @@ ExecDelete(ItemPointer tupleid, bool dodelete; dodelete = ExecBRDeleteTriggers(estate, epqstate, resultRelInfo, - tupleid); + tupleid, oldtuple); if (!dodelete) /* "do nothing" */ return NULL; @@ -352,16 +354,10 @@ ExecDelete(ItemPointer tupleid, if (resultRelInfo->ri_TrigDesc && resultRelInfo->ri_TrigDesc->trig_delete_instead_row) { - HeapTupleData tuple; bool dodelete; Assert(oldtuple != NULL); - tuple.t_data = oldtuple; - tuple.t_len = HeapTupleHeaderGetDatumLength(oldtuple); - ItemPointerSetInvalid(&(tuple.t_self)); - tuple.t_tableOid = InvalidOid; - - dodelete = ExecIRDeleteTriggers(estate, resultRelInfo, &tuple); + dodelete = ExecIRDeleteTriggers(estate, resultRelInfo, oldtuple); if (!dodelete) /* "do nothing" */ return NULL; @@ -488,7 +484,7 @@ ldelete:; (estate->es_processed)++; /* AFTER ROW DELETE Triggers */ - ExecARDeleteTriggers(estate, resultRelInfo, tupleid); + ExecARDeleteTriggers(estate, resultRelInfo, tupleid, oldtuple); /* Process RETURNING if present */ if (resultRelInfo->ri_projectReturning) @@ -512,10 +508,7 @@ ldelete:; slot = estate->es_trig_tuple_slot; if (oldtuple != NULL) { - deltuple.t_data = oldtuple; - deltuple.t_len = HeapTupleHeaderGetDatumLength(oldtuple); - ItemPointerSetInvalid(&(deltuple.t_self)); - deltuple.t_tableOid = InvalidOid; + deltuple = *oldtuple; delbuffer = InvalidBuffer; } else @@ -564,15 +557,17 @@ ldelete:; * update and oldtuple is NULL. When updating a view, oldtuple * is passed to the INSTEAD OF triggers and identifies what to * update, and tupleid is invalid. When updating a foreign table, - * both tupleid and oldtuple are NULL; the FDW has to figure out - * which row to update using data from the planSlot. + * tupleid is invalid; the FDW has to figure out which row to + * update using data from the planSlot. oldtuple is passed to + * foreign table triggers; it is NULL when the foreign table has + * no relevant triggers. * * Returns RETURNING result if any, otherwise NULL. * ---------------------------------------------------------------- */ static TupleTableSlot * ExecUpdate(ItemPointer tupleid, - HeapTupleHeader oldtuple, + HeapTuple oldtuple, TupleTableSlot *slot, TupleTableSlot *planSlot, EPQState *epqstate, @@ -609,7 +604,7 @@ ExecUpdate(ItemPointer tupleid, resultRelInfo->ri_TrigDesc->trig_update_before_row) { slot = ExecBRUpdateTriggers(estate, epqstate, resultRelInfo, - tupleid, slot); + tupleid, oldtuple, slot); if (slot == NULL) /* "do nothing" */ return NULL; @@ -622,16 +617,8 @@ ExecUpdate(ItemPointer tupleid, if (resultRelInfo->ri_TrigDesc && resultRelInfo->ri_TrigDesc->trig_update_instead_row) { - HeapTupleData oldtup; - - Assert(oldtuple != NULL); - oldtup.t_data = oldtuple; - oldtup.t_len = HeapTupleHeaderGetDatumLength(oldtuple); - ItemPointerSetInvalid(&(oldtup.t_self)); - oldtup.t_tableOid = InvalidOid; - slot = ExecIRUpdateTriggers(estate, resultRelInfo, - &oldtup, slot); + oldtuple, slot); if (slot == NULL) /* "do nothing" */ return NULL; @@ -788,7 +775,7 @@ lreplace:; (estate->es_processed)++; /* AFTER ROW UPDATE Triggers */ - ExecARUpdateTriggers(estate, resultRelInfo, tupleid, tuple, + ExecARUpdateTriggers(estate, resultRelInfo, tupleid, oldtuple, tuple, recheckIndexes); list_free(recheckIndexes); @@ -873,7 +860,8 @@ ExecModifyTable(ModifyTableState *node) TupleTableSlot *planSlot; ItemPointer tupleid = NULL; ItemPointerData tuple_ctid; - HeapTupleHeader oldtuple = NULL; + HeapTupleData oldtupdata; + HeapTuple oldtuple; /* * This should NOT get called during EvalPlanQual; we should have passed a @@ -958,6 +946,7 @@ ExecModifyTable(ModifyTableState *node) EvalPlanQualSetSlot(&node->mt_epqstate, planSlot); slot = planSlot; + oldtuple = NULL; if (junkfilter != NULL) { /* @@ -984,11 +973,21 @@ ExecModifyTable(ModifyTableState *node) * ctid!! */ tupleid = &tuple_ctid; } - else if (relkind == RELKIND_FOREIGN_TABLE) - { - /* do nothing; FDW must fetch any junk attrs it wants */ - } - else + /* + * Use the wholerow attribute, when available, to reconstruct + * the old relation tuple. + * + * Foreign table updates have a wholerow attribute when the + * relation has an AFTER ROW trigger. Note that the wholerow + * attribute does not carry system columns. Foreign table + * triggers miss seeing those, except that we know enough here + * to set t_tableOid. Quite separately from this, the FDW may + * fetch its own junk attrs to identify the row. + * + * Other relevant relkinds, currently limited to views, always + * have a wholerow attribute. + */ + else if (AttributeNumberIsValid(junkfilter->jf_junkAttNo)) { datum = ExecGetJunkAttribute(slot, junkfilter->jf_junkAttNo, @@ -997,8 +996,19 @@ ExecModifyTable(ModifyTableState *node) if (isNull) elog(ERROR, "wholerow is NULL"); - oldtuple = DatumGetHeapTupleHeader(datum); + oldtupdata.t_data = DatumGetHeapTupleHeader(datum); + oldtupdata.t_len = + HeapTupleHeaderGetDatumLength(oldtupdata.t_data); + ItemPointerSetInvalid(&(oldtupdata.t_self)); + /* Historically, view triggers see invalid t_tableOid. */ + oldtupdata.t_tableOid = + (relkind == RELKIND_VIEW) ? InvalidOid : + RelationGetRelid(resultRelInfo->ri_RelationDesc); + + oldtuple = &oldtupdata; } + else + Assert(relkind == RELKIND_FOREIGN_TABLE); } /* @@ -1334,7 +1344,11 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) } else if (relkind == RELKIND_FOREIGN_TABLE) { - /* FDW must fetch any junk attrs it wants */ + /* + * When there is an AFTER trigger, there should be a + * wholerow attribute. + */ + j->jf_junkAttNo = ExecFindJunkAttribute(j, "wholerow"); } else { diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 3728d8c418c..5dbcce3e550 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -1199,7 +1199,7 @@ static void rewriteTargetListUD(Query *parsetree, RangeTblEntry *target_rte, Relation target_relation) { - Var *var; + Var *var = NULL; const char *attrname; TargetEntry *tle; @@ -1231,7 +1231,26 @@ rewriteTargetListUD(Query *parsetree, RangeTblEntry *target_rte, fdwroutine->AddForeignUpdateTargets(parsetree, target_rte, target_relation); - return; + /* + * If we have a row-level trigger corresponding to the operation, emit + * a whole-row Var so that executor will have the "old" row to pass to + * the trigger. Alas, this misses system columns. + */ + if (target_relation->trigdesc && + ((parsetree->commandType == CMD_UPDATE && + (target_relation->trigdesc->trig_update_after_row || + target_relation->trigdesc->trig_update_before_row)) || + (parsetree->commandType == CMD_DELETE && + (target_relation->trigdesc->trig_delete_after_row || + target_relation->trigdesc->trig_delete_before_row)))) + { + var = makeWholeRowVar(target_rte, + parsetree->resultRelation, + 0, + false); + + attrname = "wholerow"; + } } else { @@ -1247,12 +1266,15 @@ rewriteTargetListUD(Query *parsetree, RangeTblEntry *target_rte, attrname = "wholerow"; } - tle = makeTargetEntry((Expr *) var, - list_length(parsetree->targetList) + 1, - pstrdup(attrname), - true); + if (var != NULL) + { + tle = makeTargetEntry((Expr *) var, + list_length(parsetree->targetList) + 1, + pstrdup(attrname), + true); - parsetree->targetList = lappend(parsetree->targetList, tle); + parsetree->targetList = lappend(parsetree->targetList, tle); + } } diff --git a/src/include/commands/trigger.h b/src/include/commands/trigger.h index 18cb128ed4d..d0b0356ba6d 100644 --- a/src/include/commands/trigger.h +++ b/src/include/commands/trigger.h @@ -147,10 +147,12 @@ extern void ExecASDeleteTriggers(EState *estate, extern bool ExecBRDeleteTriggers(EState *estate, EPQState *epqstate, ResultRelInfo *relinfo, - ItemPointer tupleid); + ItemPointer tupleid, + HeapTuple fdw_trigtuple); extern void ExecARDeleteTriggers(EState *estate, ResultRelInfo *relinfo, - ItemPointer tupleid); + ItemPointer tupleid, + HeapTuple fdw_trigtuple); extern bool ExecIRDeleteTriggers(EState *estate, ResultRelInfo *relinfo, HeapTuple trigtuple); @@ -162,10 +164,12 @@ extern TupleTableSlot *ExecBRUpdateTriggers(EState *estate, EPQState *epqstate, ResultRelInfo *relinfo, ItemPointer tupleid, + HeapTuple fdw_trigtuple, TupleTableSlot *slot); extern void ExecARUpdateTriggers(EState *estate, ResultRelInfo *relinfo, ItemPointer tupleid, + HeapTuple fdw_trigtuple, HeapTuple newtuple, List *recheckIndexes); extern TupleTableSlot *ExecIRUpdateTriggers(EState *estate, diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out index 60506e07b1c..c34c9b4df42 100644 --- a/src/test/regress/expected/foreign_data.out +++ b/src/test/regress/expected/foreign_data.out @@ -1158,6 +1158,43 @@ CREATE USER MAPPING FOR current_user SERVER s9; DROP SERVER s9 CASCADE; -- ERROR ERROR: must be owner of foreign server s9 RESET ROLE; +-- Triggers +CREATE FUNCTION dummy_trigger() RETURNS TRIGGER AS $$ + BEGIN + RETURN NULL; + END +$$ language plpgsql; +CREATE TRIGGER trigtest_before_stmt BEFORE INSERT OR UPDATE OR DELETE +ON foreign_schema.foreign_table_1 +FOR EACH STATEMENT +EXECUTE PROCEDURE dummy_trigger(); +CREATE TRIGGER trigtest_after_stmt AFTER INSERT OR UPDATE OR DELETE +ON foreign_schema.foreign_table_1 +FOR EACH STATEMENT +EXECUTE PROCEDURE dummy_trigger(); +CREATE TRIGGER trigtest_before_row BEFORE INSERT OR UPDATE OR DELETE +ON foreign_schema.foreign_table_1 +FOR EACH ROW +EXECUTE PROCEDURE dummy_trigger(); +CREATE TRIGGER trigtest_after_row AFTER INSERT OR UPDATE OR DELETE +ON foreign_schema.foreign_table_1 +FOR EACH ROW +EXECUTE PROCEDURE dummy_trigger(); +CREATE CONSTRAINT TRIGGER trigtest_constraint AFTER INSERT OR UPDATE OR DELETE +ON foreign_schema.foreign_table_1 +FOR EACH ROW +EXECUTE PROCEDURE dummy_trigger(); +ERROR: "foreign_table_1" is a foreign table +DETAIL: Foreign tables cannot have constraint triggers. +ALTER FOREIGN TABLE foreign_schema.foreign_table_1 + DISABLE TRIGGER trigtest_before_stmt; +ALTER FOREIGN TABLE foreign_schema.foreign_table_1 + ENABLE TRIGGER trigtest_before_stmt; +DROP TRIGGER trigtest_before_stmt ON foreign_schema.foreign_table_1; +DROP TRIGGER trigtest_before_row ON foreign_schema.foreign_table_1; +DROP TRIGGER trigtest_after_stmt ON foreign_schema.foreign_table_1; +DROP TRIGGER trigtest_after_row ON foreign_schema.foreign_table_1; +DROP FUNCTION dummy_trigger(); -- DROP FOREIGN TABLE DROP FOREIGN TABLE no_table; -- ERROR ERROR: foreign table "no_table" does not exist diff --git a/src/test/regress/sql/foreign_data.sql b/src/test/regress/sql/foreign_data.sql index f819eb1b8eb..0f0869ee268 100644 --- a/src/test/regress/sql/foreign_data.sql +++ b/src/test/regress/sql/foreign_data.sql @@ -470,6 +470,50 @@ CREATE USER MAPPING FOR current_user SERVER s9; DROP SERVER s9 CASCADE; -- ERROR RESET ROLE; +-- Triggers +CREATE FUNCTION dummy_trigger() RETURNS TRIGGER AS $$ + BEGIN + RETURN NULL; + END +$$ language plpgsql; + +CREATE TRIGGER trigtest_before_stmt BEFORE INSERT OR UPDATE OR DELETE +ON foreign_schema.foreign_table_1 +FOR EACH STATEMENT +EXECUTE PROCEDURE dummy_trigger(); + +CREATE TRIGGER trigtest_after_stmt AFTER INSERT OR UPDATE OR DELETE +ON foreign_schema.foreign_table_1 +FOR EACH STATEMENT +EXECUTE PROCEDURE dummy_trigger(); + +CREATE TRIGGER trigtest_before_row BEFORE INSERT OR UPDATE OR DELETE +ON foreign_schema.foreign_table_1 +FOR EACH ROW +EXECUTE PROCEDURE dummy_trigger(); + +CREATE TRIGGER trigtest_after_row AFTER INSERT OR UPDATE OR DELETE +ON foreign_schema.foreign_table_1 +FOR EACH ROW +EXECUTE PROCEDURE dummy_trigger(); + +CREATE CONSTRAINT TRIGGER trigtest_constraint AFTER INSERT OR UPDATE OR DELETE +ON foreign_schema.foreign_table_1 +FOR EACH ROW +EXECUTE PROCEDURE dummy_trigger(); + +ALTER FOREIGN TABLE foreign_schema.foreign_table_1 + DISABLE TRIGGER trigtest_before_stmt; +ALTER FOREIGN TABLE foreign_schema.foreign_table_1 + ENABLE TRIGGER trigtest_before_stmt; + +DROP TRIGGER trigtest_before_stmt ON foreign_schema.foreign_table_1; +DROP TRIGGER trigtest_before_row ON foreign_schema.foreign_table_1; +DROP TRIGGER trigtest_after_stmt ON foreign_schema.foreign_table_1; +DROP TRIGGER trigtest_after_row ON foreign_schema.foreign_table_1; + +DROP FUNCTION dummy_trigger(); + -- DROP FOREIGN TABLE DROP FOREIGN TABLE no_table; -- ERROR DROP FOREIGN TABLE IF EXISTS no_table; |