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.

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,
-		deparseReturningList(buf, root, rtindex, rel, returningList,
+					   rel->trigdesc && rel->trigdesc->trig_insert_after_row,
-							 retrieved_attrs);
+						 returningList, retrieved_attrs);
-	else
-		*retrieved_attrs = NIL;
 }
 
 /*
@@ -919,11 +918,9 @@ deparseUpdateSql(StringInfo buf, PlannerInfo *root,
 	}
 	appendStringInfoString(buf, " WHERE ctid = $1");
 
-	if (returningList)
+	deparseReturningList(buf, root, rtindex, rel,
-		deparseReturningList(buf, root, rtindex, rel, returningList,
+					   rel->trigdesc && rel->trigdesc->trig_update_after_row,
-							 retrieved_attrs);
+						 returningList, retrieved_attrs);
-	else
-		*retrieved_attrs = NIL;
 }
 
 /*
@@ -943,34 +940,48 @@ deparseDeleteSql(StringInfo buf, PlannerInfo *root,
 	deparseRelation(buf, rel);
 	appendStringInfoString(buf, " WHERE ctid = $1");
 
-	if (returningList)
+	deparseReturningList(buf, root, rtindex, rel,
-		deparseReturningList(buf, root, rtindex, rel, returningList,
+					   rel->trigdesc && rel->trigdesc->trig_delete_after_row,
-							 retrieved_attrs);
+						 returningList, retrieved_attrs);
-	else
-		*retrieved_attrs = NIL;
 }
 
 /*
- * 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;
 
-	/*
+	if (trig_after_row)
-	 * We need the attrs mentioned in the query's RETURNING list.
+	{
-	 */
+		/* whole-row reference acquires all non-system columns */
-	attrs_used = NULL;
+		attrs_used =
-	pull_varattnos((Node *) returningList, rtindex,
+			bms_make_singleton(0 - FirstLowInvalidHeapAttributeNumber);
-				   &attrs_used);
+	}
 
-	appendStringInfoString(buf, " RETURNING ");
+	if (returningList != NIL)
-	deparseTargetList(buf, root, rtindex, rel, attrs_used,
+	{
-					  retrieved_attrs);
+		/*
+		 * 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;
 }
 
 /*

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)
+

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);
 }
 

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;

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
+     query has a <literal>RETURNING</> clause or the foreign table has
-     to optimize away returning some or all columns depending on the contents
+     an <literal>AFTER ROW</> trigger.  Triggers require all columns, but the
-     of the <literal>RETURNING</> clause.  However, some slot must be
+     FDW could choose to optimize away returning some or all columns depending
-     returned to indicate success, or the query's reported row count will be
+     on the contents of the <literal>RETURNING</> clause.  Regardless, some
-     wrong.
+     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
+     query has a <literal>RETURNING</> clause or the foreign table has
-     to optimize away returning some or all columns depending on the contents
+     an <literal>AFTER ROW</> trigger.  Triggers require all columns, but the
-     of the <literal>RETURNING</> clause.  However, some slot must be
+     FDW could choose to optimize away returning some or all columns depending
-     returned to indicate success, or the query's reported row count will be
+     on the contents of the <literal>RETURNING</> clause.  Regardless, some
-     wrong.
+     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
+     query has a <literal>RETURNING</> clause or the foreign table has
-     to optimize away returning some or all columns depending on the contents
+     an <literal>AFTER ROW</> trigger.  Triggers require all columns, but the
-     of the <literal>RETURNING</> clause.  However, some slot must be
+     FDW could choose to optimize away returning some or all columns depending
-     returned to indicate success, or the query's reported row count will be
+     on the contents of the <literal>RETURNING</> clause.  Regardless, some
-     wrong.
+     slot must be returned to indicate success, or the query's reported row
+     count will be wrong.
     </para>
 
     <para>

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
+   trigger will be associated with the specified table, view, or foreign table
-   execute the specified function <replaceable
+   and will execute the specified
-   class="parameter">function_name</replaceable> when certain events occur.
+   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 foreign tables</entry>
-      <entry align="center">Tables and views</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 foreign tables</entry>
-      <entry align="center">Tables and views</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
+   Constraint triggers must be <literal>AFTER ROW</> triggers on tables.  They
-   be fired either at the end of the statement causing the triggering event,
+   can be fired either at the end of the statement causing the triggering
-   or at the end of the containing transaction; in the latter case they are
+   event, or at the end of the containing transaction; in the latter case they
-   said to be <firstterm>deferred</>.  A pending deferred-trigger firing can
+   are said to be <firstterm>deferred</>.  A pending deferred-trigger firing
-   also be forced to happen immediately by using <command>SET CONSTRAINTS</>.
+   can also be forced to happen immediately by using <command>SET
-   Constraint triggers are expected to raise an exception when the constraints
+   CONSTRAINTS</>.  Constraint triggers are expected to raise an exception
-   they implement are violated.
+   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
+      The name (optionally schema-qualified) of the table, view, or foreign
-      is for.
+      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

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
+    On tables and foreign tables, triggers can be defined to execute either
-    <command>INSERT</command>, <command>UPDATE</command>, or
+    before or after any <command>INSERT</command>, <command>UPDATE</command>,
-    <command>DELETE</command> operation, either once per modified row,
+    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
+    These types of triggers may only be defined on tables and foreign tables.
-    <literal>INSTEAD OF</> triggers may only be defined on views, and fire
+    Row-level <literal>INSTEAD OF</> triggers may only be defined on views,
-    immediately as each row in the view is identified as needing to be
+    and fire immediately as each row in the view is identified as needing to
-    operated on.
+    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>

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:

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,
+	Assert(HeapTupleIsValid(fdw_trigtuple) ^ ItemPointerIsValid(tupleid));
-								   LockTupleExclusive, &newSlot);
+	if (fdw_trigtuple == NULL)
-	if (trigtuple == NULL)
+	{
-		return false;
+		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,
+		HeapTuple	trigtuple;
-												   NULL,
+
-												   relinfo,
+		Assert(HeapTupleIsValid(fdw_trigtuple) ^ ItemPointerIsValid(tupleid));
-												   tupleid,
+		if (fdw_trigtuple == NULL)
-												   LockTupleExclusive,
+			trigtuple = GetTupleForTrigger(estate,
-												   NULL);
+										   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 */
+	Assert(HeapTupleIsValid(fdw_trigtuple) ^ ItemPointerIsValid(tupleid));
-	trigtuple = GetTupleForTrigger(estate, epqstate, relinfo, tupleid,
+	if (fdw_trigtuple == NULL)
-								   lockmode, &newSlot);
+	{
-	if (trigtuple == NULL)
+		/* get a copy of the on-disk tuple we are planning to update */
-		return NULL;			/* cancel the update action */
+		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,
+		HeapTuple	trigtuple;
-												   NULL,
+
-												   relinfo,
+		Assert(HeapTupleIsValid(fdw_trigtuple) ^ ItemPointerIsValid(tupleid));
-												   tupleid,
+		if (fdw_trigtuple == NULL)
-												   LockTupleExclusive,
+			trigtuple = GetTupleForTrigger(estate,
-												   NULL);
+										   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
+ * status bits and up to two tuple CTIDs.  Each event record also has an
- * associated AfterTriggerSharedData that is shared across all instances
+ * associated AfterTriggerSharedData that is shared across all instances of
- * of similar events within a "chunk".
+ * similar events within a "chunk".
  *
- * We arrange not to waste storage on ate_ctid2 for non-update events.
+ * For row-level triggers, we arrange not to waste storage on unneeded ctid
- * We could go further and not store either ctid for statement-level triggers,
+ * fields.  Updates of regular tables use two; inserts and deletes of regular
- * but that seems unlikely to be worth the trouble.
+ * 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				0x10000000
-#define AFTER_TRIGGER_DONE				0x20000000
+#define AFTER_TRIGGER_IN_PROGRESS		0x20000000
-#define AFTER_TRIGGER_IN_PROGRESS		0x40000000
+/* 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) ? \
+	(((evt)->ate_flags & AFTER_TRIGGER_TUP_BITS) == AFTER_TRIGGER_2CTID ? \
-	 sizeof(AfterTriggerEventData) : sizeof(AfterTriggerEventDataOneCtid))
+	 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));
+		case AFTER_TRIGGER_FDW_FETCH:
-		if (!heap_fetch(rel, SnapshotAny, &tuple1, &buffer1, false, NULL))
+			{
-			elog(ERROR, "failed to fetch tuple1 for AFTER trigger");
+				Tuplestorestate *fdw_tuplestore = GetCurrentFDWTuplestore();
-		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 (!tuplestore_gettupleslot(fdw_tuplestore, true, false,
-	if ((event->ate_flags & AFTER_TRIGGER_2CTIDS) &&
+											 trig_tuple_slot1))
-		ItemPointerIsValid(&(event->ate_ctid2)))
+					elog(ERROR, "failed to fetch tuple1 for AFTER trigger");
-	{
+
-		ItemPointerCopy(&(event->ate_ctid2), &(tuple2.t_self));
+				if ((evtshared->ats_event & TRIGGER_EVENT_OPMASK) ==
-		if (!heap_fetch(rel, SnapshotAny, &tuple2, &buffer2, false, NULL))
+					TRIGGER_EVENT_UPDATE &&
-			elog(ERROR, "failed to fetch tuple2 for AFTER trigger");
+					!tuplestore_gettupleslot(fdw_tuplestore, true, false,
-		LocTriggerData.tg_newtuple = &tuple2;
+											 trig_tuple_slot2))
-		LocTriggerData.tg_newtuplebuf = buffer2;
+					elog(ERROR, "failed to fetch tuple2 for AFTER trigger");
-	}
+			}
-	else
+			/* fall through */
-	{
+		case AFTER_TRIGGER_FDW_REUSE:
-		LocTriggerData.tg_newtuple = NULL;
+			/*
-		LocTriggerData.tg_newtuplebuf = InvalidBuffer;
+			 * 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

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
+ *		foreign table, tupleid is invalid; the FDW has to figure out
- *		to figure out which row to delete using data from the planSlot.
+ *		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;
+		dodelete = ExecIRDeleteTriggers(estate, resultRelInfo, oldtuple);
-		tuple.t_len = HeapTupleHeaderGetDatumLength(oldtuple);
-		ItemPointerSetInvalid(&(tuple.t_self));
-		tuple.t_tableOid = InvalidOid;
-
-		dodelete = ExecIRDeleteTriggers(estate, resultRelInfo, &tuple);
 
 		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 = *oldtuple;
-				deltuple.t_len = HeapTupleHeaderGetDatumLength(oldtuple);
-				ItemPointerSetInvalid(&(deltuple.t_self));
-				deltuple.t_tableOid = InvalidOid;
 				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
+ *		tupleid is invalid; the FDW has to figure out which row to
- *		which row to update using data from the planSlot.
+ *		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)
+				/*
-				{
+				 * Use the wholerow attribute, when available, to reconstruct
-					/* do nothing; FDW must fetch any junk attrs it wants */
+				 * the old relation tuple.
-				}
+				 *
-				else
+				 * 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
 					{

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,
+	if (var != NULL)
-						  list_length(parsetree->targetList) + 1,
+	{
-						  pstrdup(attrname),
+		tle = makeTargetEntry((Expr *) var,
-						  true);
+							  list_length(parsetree->targetList) + 1,
+							  pstrdup(attrname),
+							  true);
 
-	parsetree->targetList = lappend(parsetree->targetList, tle);
+		parsetree->targetList = lappend(parsetree->targetList, tle);
+	}
 }
 
 

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,

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

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;