Fix creation of resjunk tlist entries for inherited mixed UPDATE/DELETE.

rewriteTargetListUD's processing is dependent on the relkind of the query's
target table.  That was fine at the time it was made to act that way, even
for queries on inheritance trees, because all tables in an inheritance tree
would necessarily be plain tables.  However, the 9.5 feature addition
allowing some members of an inheritance tree to be foreign tables broke the
assumption that rewriteTargetListUD's output tlist could be applied to all
child tables with nothing more than column-number mapping.  This led to
visible failures if foreign child tables had row-level triggers, and would
also break in cases where child tables belonged to FDWs that used methods
other than CTID for row identification.

To fix, delay running rewriteTargetListUD until after the planner has
expanded inheritance, so that it is applied separately to the (already
mapped) tlist for each child table.  We can conveniently call it from
preprocess_targetlist.  Refactor associated code slightly to avoid the
need to heap_open the target relation multiple times during
preprocess_targetlist.  (The APIs remain a bit ugly, particularly around
the point of which steps scribble on parse->targetList and which don't.
But avoiding such scribbling would require a change in FDW callback APIs,
which is more pain than it's worth.)

Also fix ExecModifyTable to ensure that "tupleid" is reset to NULL when
we transition from rows providing a CTID to rows that don't.  (That's
really an independent bug, but it manifests in much the same cases.)

Add a regression test checking one manifestation of this problem, which
was that row-level triggers on a foreign child table did not work right.

Back-patch to 9.5 where the problem was introduced.

Etsuro Fujita, reviewed by Ildus Kurbangaliev and Ashutosh Bapat

Discussion: https://postgr.es/m/20170514150525.0346ba72@postgrespro.ru

2 files changed, 83 insertions, 0 deletions

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 89634b363f0..5e311f5956e 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -3592,6 +3592,71 @@ fetch from c;
 update bar set f2 = null where current of c;
 ERROR:  WHERE CURRENT OF is not supported for this table type
 rollback;
+-- Test that UPDATE/DELETE with inherited target works with row-level triggers
+CREATE TRIGGER trig_row_before
+BEFORE UPDATE OR DELETE ON bar2
+FOR EACH ROW EXECUTE PROCEDURE trigger_data(23,'skidoo');
+CREATE TRIGGER trig_row_after
+AFTER UPDATE OR DELETE ON bar2
+FOR EACH ROW EXECUTE PROCEDURE trigger_data(23,'skidoo');
+explain (verbose, costs off)
+update bar set f2 = f2 + 100;
+                                      QUERY PLAN                                      
+--------------------------------------------------------------------------------------
+ Update on public.bar
+   Update on public.bar
+   Foreign Update on public.bar2
+     Remote SQL: UPDATE public.loct2 SET f2 = $2 WHERE ctid = $1 RETURNING f1, f2, f3
+   ->  Seq Scan on public.bar
+         Output: bar.f1, (bar.f2 + 100), bar.ctid
+   ->  Foreign Scan on public.bar2
+         Output: bar2.f1, (bar2.f2 + 100), bar2.f3, bar2.ctid, bar2.*
+         Remote SQL: SELECT f1, f2, f3, ctid FROM public.loct2 FOR UPDATE
+(9 rows)
+
+update bar set f2 = f2 + 100;
+NOTICE:  trig_row_before(23, skidoo) BEFORE ROW UPDATE ON bar2
+NOTICE:  OLD: (3,233,33),NEW: (3,333,33)
+NOTICE:  trig_row_before(23, skidoo) BEFORE ROW UPDATE ON bar2
+NOTICE:  OLD: (4,244,44),NEW: (4,344,44)
+NOTICE:  trig_row_before(23, skidoo) BEFORE ROW UPDATE ON bar2
+NOTICE:  OLD: (7,177,77),NEW: (7,277,77)
+NOTICE:  trig_row_after(23, skidoo) AFTER ROW UPDATE ON bar2
+NOTICE:  OLD: (3,233,33),NEW: (3,333,33)
+NOTICE:  trig_row_after(23, skidoo) AFTER ROW UPDATE ON bar2
+NOTICE:  OLD: (4,244,44),NEW: (4,344,44)
+NOTICE:  trig_row_after(23, skidoo) AFTER ROW UPDATE ON bar2
+NOTICE:  OLD: (7,177,77),NEW: (7,277,77)
+explain (verbose, costs off)
+delete from bar where f2 < 400;
+                                         QUERY PLAN                                          
+---------------------------------------------------------------------------------------------
+ Delete on public.bar
+   Delete on public.bar
+   Foreign Delete on public.bar2
+     Remote SQL: DELETE FROM public.loct2 WHERE ctid = $1 RETURNING f1, f2, f3
+   ->  Seq Scan on public.bar
+         Output: bar.ctid
+         Filter: (bar.f2 < 400)
+   ->  Foreign Scan on public.bar2
+         Output: bar2.ctid, bar2.*
+         Remote SQL: SELECT f1, f2, f3, ctid FROM public.loct2 WHERE ((f2 < 400)) FOR UPDATE
+(10 rows)
+
+delete from bar where f2 < 400;
+NOTICE:  trig_row_before(23, skidoo) BEFORE ROW DELETE ON bar2
+NOTICE:  OLD: (3,333,33)
+NOTICE:  trig_row_before(23, skidoo) BEFORE ROW DELETE ON bar2
+NOTICE:  OLD: (4,344,44)
+NOTICE:  trig_row_before(23, skidoo) BEFORE ROW DELETE ON bar2
+NOTICE:  OLD: (7,277,77)
+NOTICE:  trig_row_after(23, skidoo) AFTER ROW DELETE ON bar2
+NOTICE:  OLD: (3,333,33)
+NOTICE:  trig_row_after(23, skidoo) AFTER ROW DELETE ON bar2
+NOTICE:  OLD: (4,344,44)
+NOTICE:  trig_row_after(23, skidoo) AFTER ROW DELETE ON bar2
+NOTICE:  OLD: (7,277,77)
+-- cleanup
 drop table foo cascade;
 NOTICE:  drop cascades to foreign table foo2
 drop table bar cascade;
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 45ef1f29315..698be8e2917 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -825,6 +825,24 @@ fetch from c;
 update bar set f2 = null where current of c;
 rollback;
 
+-- Test that UPDATE/DELETE with inherited target works with row-level triggers
+CREATE TRIGGER trig_row_before
+BEFORE UPDATE OR DELETE ON bar2
+FOR EACH ROW EXECUTE PROCEDURE trigger_data(23,'skidoo');
+
+CREATE TRIGGER trig_row_after
+AFTER UPDATE OR DELETE ON bar2
+FOR EACH ROW EXECUTE PROCEDURE trigger_data(23,'skidoo');
+
+explain (verbose, costs off)
+update bar set f2 = f2 + 100;
+update bar set f2 = f2 + 100;
+
+explain (verbose, costs off)
+delete from bar where f2 < 400;
+delete from bar where f2 < 400;
+
+-- cleanup
 drop table foo cascade;
 drop table bar cascade;
 drop table loct1;