adjust-inherited-update-tlist-v2.patch

text/plain

Filename: adjust-inherited-update-tlist-v2.patch
Type: text/plain
Part: 0
Message: Re: Bug in ExecModifyTable function and trigger issues for foreign tables

Patch

Format: context
Series: patch v2
File+
contrib/postgres_fdw/expected/postgres_fdw.out 109 0
contrib/postgres_fdw/sql/postgres_fdw.sql 28 0
src/backend/executor/nodeModifyTable.c 1 0
src/backend/optimizer/prep/prepunion.c 86 0
src/backend/rewrite/rewriteHandler.c 1 0
src/include/rewrite/rewriteHandler.h 2 0
*** a/contrib/postgres_fdw/expected/postgres_fdw.out
--- b/contrib/postgres_fdw/expected/postgres_fdw.out
***************
*** 6924,6929 **** update bar set f2 = f2 + 100 returning *;
--- 6924,7038 ----
    7 | 277
  (6 rows)
  
+ -- 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,333,33),NEW: (3,433,33)
+ NOTICE:  trig_row_before(23, skidoo) BEFORE ROW UPDATE ON bar2
+ NOTICE:  OLD: (4,344,44),NEW: (4,444,44)
+ NOTICE:  trig_row_before(23, skidoo) BEFORE ROW UPDATE ON bar2
+ NOTICE:  OLD: (7,277,77),NEW: (7,377,77)
+ NOTICE:  trig_row_after(23, skidoo) AFTER ROW UPDATE ON bar2
+ NOTICE:  OLD: (3,333,33),NEW: (3,433,33)
+ NOTICE:  trig_row_after(23, skidoo) AFTER ROW UPDATE ON bar2
+ NOTICE:  OLD: (4,344,44),NEW: (4,444,44)
+ NOTICE:  trig_row_after(23, skidoo) AFTER ROW UPDATE ON bar2
+ NOTICE:  OLD: (7,277,77),NEW: (7,377,77)
+ explain (verbose, costs off)
+ update bar set (f2,f1) = (select f1, f2 from bar2 where f3 = 77) where f1 = 7;
+                                           QUERY PLAN                                           
+ -----------------------------------------------------------------------------------------------
+  Update on public.bar
+    Update on public.bar
+    Foreign Update on public.bar2
+      Remote SQL: UPDATE public.loct2 SET f1 = $2, f2 = $3 WHERE ctid = $1 RETURNING f1, f2, f3
+    InitPlan 1 (returns $0,$1)
+      ->  Foreign Scan on public.bar2 bar2_1
+            Output: bar2_1.f1, bar2_1.f2
+            Remote SQL: SELECT f1, f2 FROM public.loct2 WHERE ((f3 = 77))
+    ->  Seq Scan on public.bar
+          Output: $1, $0, NULL::record, bar.ctid
+          Filter: (bar.f1 = 7)
+    ->  Foreign Scan on public.bar2
+          Output: $1, $0, bar2.f3, NULL::record, bar2.ctid, bar2.*
+          Remote SQL: SELECT f1, f2, f3, ctid FROM public.loct2 WHERE ((f1 = 7)) FOR UPDATE
+ (14 rows)
+ 
+ update bar set (f2,f1) = (select f1, f2 from bar2 where f3 = 77) where f1 = 7;
+ NOTICE:  trig_row_before(23, skidoo) BEFORE ROW UPDATE ON bar2
+ NOTICE:  OLD: (7,377,77),NEW: (377,7,77)
+ NOTICE:  trig_row_after(23, skidoo) AFTER ROW UPDATE ON bar2
+ NOTICE:  OLD: (7,377,77),NEW: (377,7,77)
+ explain (verbose, costs off)
+ update bar o set (f2,f1) = (select f1, f2 from bar2 i where i.f1 = o.f1 and i.f2 = o.f2 and i.f3 = 77) where f2 = 7;
+                                                            QUERY PLAN                                                            
+ ---------------------------------------------------------------------------------------------------------------------------------
+  Update on public.bar o
+    Update on public.bar o
+    Foreign Update on public.bar2 o_1
+      Remote SQL: UPDATE public.loct2 SET f1 = $2, f2 = $3 WHERE ctid = $1 RETURNING f1, f2, f3
+    ->  Seq Scan on public.bar o
+          Output: $3, $2, (SubPlan 1 (returns $2,$3)), o.ctid
+          Filter: (o.f2 = 7)
+          SubPlan 1 (returns $2,$3)
+            ->  Foreign Scan on public.bar2 i
+                  Output: i.f1, i.f2
+                  Remote SQL: SELECT f1, f2 FROM public.loct2 WHERE ((f1 = $1::integer)) AND ((f2 = $2::integer)) AND ((f3 = 77))
+    ->  Foreign Scan on public.bar2 o_1
+          Output: $3, $2, o_1.f3, (SubPlan 1 (returns $2,$3)), o_1.ctid, o_1.*
+          Remote SQL: SELECT f1, f2, f3, ctid FROM public.loct2 WHERE ((f2 = 7)) FOR UPDATE
+ (14 rows)
+ 
+ update bar o set (f2,f1) = (select f1, f2 from bar2 i where i.f1 = o.f1 and i.f2 = o.f2 and i.f3 = 77) where f2 = 7;
+ NOTICE:  trig_row_before(23, skidoo) BEFORE ROW UPDATE ON bar2
+ NOTICE:  OLD: (377,7,77),NEW: (7,377,77)
+ NOTICE:  trig_row_after(23, skidoo) AFTER ROW UPDATE ON bar2
+ NOTICE:  OLD: (377,7,77),NEW: (7,377,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: (7,377,77)
+ NOTICE:  trig_row_after(23, skidoo) AFTER ROW DELETE ON bar2
+ NOTICE:  OLD: (7,377,77)
+ -- cleanup
+ DROP TRIGGER trig_row_before ON bar2;
+ DROP TRIGGER trig_row_after ON bar2;
  drop table foo cascade;
  NOTICE:  drop cascades to foreign table foo2
  drop table bar cascade;
*** a/contrib/postgres_fdw/sql/postgres_fdw.sql
--- b/contrib/postgres_fdw/sql/postgres_fdw.sql
***************
*** 1609,1614 **** explain (verbose, costs off)
--- 1609,1642 ----
  update bar set f2 = f2 + 100 returning *;
  update bar set f2 = f2 + 100 returning *;
  
+ -- 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)
+ update bar set (f2,f1) = (select f1, f2 from bar2 where f3 = 77) where f1 = 7;
+ update bar set (f2,f1) = (select f1, f2 from bar2 where f3 = 77) where f1 = 7;
+ 
+ explain (verbose, costs off)
+ update bar o set (f2,f1) = (select f1, f2 from bar2 i where i.f1 = o.f1 and i.f2 = o.f2 and i.f3 = 77) where f2 = 7;
+ update bar o set (f2,f1) = (select f1, f2 from bar2 i where i.f1 = o.f1 and i.f2 = o.f2 and i.f3 = 77) where f2 = 7;
+ 
+ explain (verbose, costs off)
+ delete from bar where f2 < 400;
+ delete from bar where f2 < 400;
+ 
+ -- cleanup
+ DROP TRIGGER trig_row_before ON bar2;
+ DROP TRIGGER trig_row_after ON bar2;
  drop table foo cascade;
  drop table bar cascade;
  drop table loct1;
*** a/src/backend/executor/nodeModifyTable.c
--- b/src/backend/executor/nodeModifyTable.c
***************
*** 1538,1543 **** ExecModifyTable(ModifyTableState *node)
--- 1538,1544 ----
  		EvalPlanQualSetSlot(&node->mt_epqstate, planSlot);
  		slot = planSlot;
  
+ 		tupleid = NULL;
  		oldtuple = NULL;
  		if (junkfilter != NULL)
  		{
*** a/src/backend/optimizer/prep/prepunion.c
--- b/src/backend/optimizer/prep/prepunion.c
***************
*** 47,52 ****
--- 47,53 ----
  #include "optimizer/tlist.h"
  #include "parser/parse_coerce.h"
  #include "parser/parsetree.h"
+ #include "rewrite/rewriteHandler.h"
  #include "utils/lsyscache.h"
  #include "utils/rel.h"
  #include "utils/selfuncs.h"
***************
*** 110,115 **** static Node *adjust_appendrel_attrs_mutator(Node *node,
--- 111,117 ----
  static Relids adjust_relid_set(Relids relids, Index oldrelid, Index newrelid);
  static List *adjust_inherited_tlist(List *tlist,
  					   AppendRelInfo *context);
+ static void rewrite_inherited_tlist(Query *parse, RangeTblEntry *child_rte);
  
  
  /*
***************
*** 1807,1812 **** adjust_appendrel_attrs(PlannerInfo *root, Node *node, AppendRelInfo *appinfo)
--- 1809,1827 ----
  				newnode->targetList =
  					adjust_inherited_tlist(newnode->targetList,
  										   appinfo);
+ 			/* And fix junk tlist entries for UPDATE/DELETE too, if needed */
+ 			if (newnode->commandType == CMD_UPDATE ||
+ 				newnode->commandType == CMD_DELETE)
+ 			{
+ 				RangeTblEntry *childrte;
+ 				RangeTblEntry *parentrte;
+ 
+ 				childrte = planner_rt_fetch(appinfo->child_relid, root);
+ 				parentrte = planner_rt_fetch(appinfo->parent_relid, root);
+ 				if (childrte->relkind == RELKIND_FOREIGN_TABLE ||
+ 					parentrte->relkind == RELKIND_FOREIGN_TABLE)
+ 					rewrite_inherited_tlist(newnode, childrte);
+ 			}
  		}
  		result = (Node *) newnode;
  	}
***************
*** 2140,2145 **** adjust_inherited_tlist(List *tlist, AppendRelInfo *context)
--- 2155,2231 ----
  }
  
  /*
+  * Rewrite the targetlist of the specified query
+  *	  Replace resjunk entry(s) in the targetlist added by rewriteTargetListUD
+  *	  for the parent table with one(s) for the child table.
+  *
+  * The targetlist has already been through expression_tree_mutator;
+  * therefore the TargetEntry nodes are fresh copies that it's okay to
+  * scribble on.
+  */
+ static void
+ rewrite_inherited_tlist(Query *parse, RangeTblEntry *child_rte)
+ {
+ 	ListCell   *tl;
+ 	List	   *new_tlist = NIL;
+ 	List	   *junk_tlist = NIL;
+ 	Relation	child_relation;
+ 	int			next_junk_attrno;
+ 
+ 	/*
+ 	 * Open the child relation; we need not lock it since it was already
+ 	 * locked by expand_inherited_rtentry.
+ 	 */
+ 	child_relation = heap_open(child_rte->relid, NoLock);
+ 
+ 	/*
+ 	 * Create a new tlist without resjunk entries; resjunk entries other
+ 	 * than one(s) added by rewriteTargetListUD are tossed into a separate
+ 	 * list; then appended to the new tlist.
+ 	 */
+ 	foreach(tl, parse->targetList)
+ 	{
+ 		TargetEntry *tle = (TargetEntry *) lfirst(tl);
+ 
+ 		if (tle->resjunk)
+ 		{
+ 			/*
+ 			 * If the resjunk entry has a resname, it should have been added
+ 			 * by rewriteTargetListUD, so ignore it.
+ 			 */
+ 			if (!tle->resname)
+ 				junk_tlist = lappend(junk_tlist, tle);
+ 			continue;
+ 		}
+ 
+ 		new_tlist = lappend(new_tlist, tle);
+ 	}
+ 
+ 	next_junk_attrno = list_length(new_tlist) + 1;
+ 	foreach(tl, junk_tlist)
+ 	{
+ 		TargetEntry *tle = (TargetEntry *) lfirst(tl);
+ 
+ 		/* Get the resno right */
+ 		if (tle->resno != next_junk_attrno)
+ 		{
+ 			tle->resno = next_junk_attrno;
+ 		}
+ 		new_tlist = lappend(new_tlist, tle);
+ 		next_junk_attrno++;
+ 	}
+ 
+ 	/* Replace the query's targetlist with the new tlist */
+ 	parse->targetList = new_tlist;
+ 
+ 	/* And add resjunk entry(s) needed for the child table */
+ 	rewriteTargetListUD(parse, child_rte, child_relation);
+ 
+ 	/* Close the child relation, but keep the lock */
+ 	heap_close(child_relation, NoLock);
+ }
+ 
+ /*
   * adjust_appendrel_attrs_multilevel
   *	  Apply Var translations from a toplevel appendrel parent down to a child.
   *
*** a/src/backend/rewrite/rewriteHandler.c
--- b/src/backend/rewrite/rewriteHandler.c
***************
*** 72,79 **** static TargetEntry *process_matched_tle(TargetEntry *src_tle,
  static Node *get_assignment_input(Node *node);
  static void rewriteValuesRTE(RangeTblEntry *rte, Relation target_relation,
  				 List *attrnos);
- static void rewriteTargetListUD(Query *parsetree, RangeTblEntry *target_rte,
- 					Relation target_relation);
  static void markQueryForLocking(Query *qry, Node *jtnode,
  					LockClauseStrength strength, LockWaitPolicy waitPolicy,
  					bool pushedDown);
--- 72,77 ----
***************
*** 1269,1275 **** rewriteValuesRTE(RangeTblEntry *rte, Relation target_relation, List *attrnos)
   * For UPDATE queries, this is applied after rewriteTargetListIU.  The
   * ordering isn't actually critical at the moment.
   */
! static void
  rewriteTargetListUD(Query *parsetree, RangeTblEntry *target_rte,
  					Relation target_relation)
  {
--- 1267,1273 ----
   * For UPDATE queries, this is applied after rewriteTargetListIU.  The
   * ordering isn't actually critical at the moment.
   */
! void
  rewriteTargetListUD(Query *parsetree, RangeTblEntry *target_rte,
  					Relation target_relation)
  {
*** a/src/include/rewrite/rewriteHandler.h
--- b/src/include/rewrite/rewriteHandler.h
***************
*** 23,28 **** extern void AcquireRewriteLocks(Query *parsetree,
--- 23,30 ----
  					bool forUpdatePushedDown);
  
  extern Node *build_column_default(Relation rel, int attrno);
+ extern void rewriteTargetListUD(Query *parsetree, RangeTblEntry *target_rte,
+ 					Relation target_relation);
  extern Query *get_view_query(Relation view);
  extern const char *view_query_is_auto_updatable(Query *viewquery,
  							 bool check_cols);