Revert fix-rewrite-tlist-v4.patch.

arssher · arssher · commit 5bf12d0a64ff · 2017-11-24T17:20:56.000+03:00
It breaks in_memory regression test: now junk column is always there. Probably
this is fine, but I don't understand the internals very well and won't risk
here. Moreover, currently we don't need this patch anymore.
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -7022,65 +7022,6 @@ update bar set f2 = f2 + 100 returning *;
   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)
-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;
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -1656,26 +1656,6 @@ explain (verbose, costs off)
 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)
-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;
diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml
@@ -428,13 +428,11 @@ AddForeignUpdateTargets (Query *parsetree,
      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.  Also, as it assumes that those
-     expressions have already been simplified enough to execute,
-     apply <function>eval_const_expressions</> if necessary.
+     generate junk columns of these names.
     </para>
 
     <para>
-     Although this function is called in the planner, the
+     This function is called in the rewriter, not the planner, so the
      information available is a bit different from that available to the
      planning routines.
      <literal>parsetree</> is the parse tree for the <command>UPDATE</> or
diff --git a/doc/src/sgml/rules.sgml b/doc/src/sgml/rules.sgml
@@ -167,12 +167,12 @@
 
     <para>
         <command>DELETE</command> commands don't need a normal target list
-        because they don't produce any result.  Instead, the planner
+        because they don't produce any result.  Instead, the rule system
         adds a special <acronym>CTID</> entry to the empty target list,
         to allow the executor to find the row to be deleted.
         (<acronym>CTID</> is added when the result relation is an ordinary
-        table.  If it is a view, a whole-row variable is added instead, by
-        the rule system, as described in <xref linkend="rules-views-update">.)
+        table.  If it is a view, a whole-row variable is added instead,
+        as described in <xref linkend="rules-views-update">.)
     </para>
 
     <para>
@@ -194,7 +194,7 @@
         column = expression</literal> part of the command.  The planner will
         handle missing columns by inserting expressions that copy the values
         from the old row into the new one.  Just as for <command>DELETE</>,
-        a <acronym>CTID</> or whole-row variable is added so that
+        the rule system adds a <acronym>CTID</> or whole-row variable so that
         the executor can identify the old row to be updated.
     </para>
 
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
@@ -1687,7 +1687,6 @@ ExecModifyTable(PlanState *pstate)
 		EvalPlanQualSetSlot(&node->mt_epqstate, planSlot);
 		slot = planSlot;
 
-		tupleid = NULL;
 		oldtuple = NULL;
 		if (junkfilter != NULL)
 		{
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
@@ -1486,7 +1486,7 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
 				 double tuple_fraction)
 {
 	Query	   *parse = root->parse;
-	List	   *tlist;
+	List	   *tlist = parse->targetList;
 	int64		offset_est = 0;
 	int64		count_est = 0;
 	double		limit_tuples = -1.0;
@@ -1616,7 +1616,7 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
 		}
 
 		/* Preprocess targetlist */
-		tlist = preprocess_targetlist(root);
+		tlist = preprocess_targetlist(root, tlist);
 
 		if (parse->onConflict)
 			parse->onConflict->onConflictSet =
diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c
@@ -4,21 +4,20 @@
  *	  Routines to preprocess the parse tree target list
  *
  * For INSERT and UPDATE queries, the targetlist must contain an entry for
- * each attribute of the target relation in the correct order.  For UPDATE and
- * DELETE queries, it must also contain a junk tlist entry needed to allow the
- * executor to identify the physical locations of the rows to be updated or
- * deleted.  For all query types, we may need to add junk tlist entries for
- * Vars used in the RETURNING list and row ID information needed for SELECT
- * FOR UPDATE locking and/or EvalPlanQual checking.
+ * each attribute of the target relation in the correct order.  For all query
+ * types, we may need to add junk tlist entries for Vars used in the RETURNING
+ * list and row ID information needed for SELECT FOR UPDATE locking and/or
+ * EvalPlanQual checking.
  *
- * The rewriter's rewriteTargetListIU routine also does preprocessing of the
- * targetlist.  The division of labor between here and there is partially
- * historical, but it's not entirely arbitrary.  In particular, consider an
- * UPDATE across an inheritance tree.  What the rewriter does need be done
- * only once (because it depends only on the properties of the parent
- * relation).  What's done here has to be done over again for each child
- * relation, because it depends on the column list of the child, which might
- * have more columns and/or a different column order than the parent.
+ * The rewriter's rewriteTargetListIU and rewriteTargetListUD routines
+ * also do preprocessing of the targetlist.  The division of labor between
+ * here and there is partially historical, but it's not entirely arbitrary.
+ * In particular, consider an UPDATE across an inheritance tree.  What the
+ * rewriter does need be done only once (because it depends only on the
+ * properties of the parent relation).  What's done here has to be done over
+ * again for each child relation, because it depends on the column list of
+ * the child, which might have more columns and/or a different column order
+ * than the parent.
  *
  * The fact that rewriteTargetListIU sorts non-resjunk tlist entries by column
  * position, which expand_targetlist depends on, violates the above comment
@@ -42,9 +41,7 @@
 #include "access/heapam.h"
 #include "access/sysattr.h"
 #include "catalog/pg_type.h"
-#include "foreign/fdwapi.h"
 #include "nodes/makefuncs.h"
-#include "optimizer/clauses.h"
 #include "optimizer/prep.h"
 #include "optimizer/tlist.h"
 #include "optimizer/var.h"
@@ -55,8 +52,6 @@
 
 static List *expand_targetlist(List *tlist, int command_type,
 				  Index result_relation, List *range_table);
-static void rewrite_targetlist(Query *parse,
-				   Index result_relation, List *range_table);
 
 
 /*
@@ -66,13 +61,12 @@ static void rewrite_targetlist(Query *parse,
  *	  Returns the new targetlist.
  */
 List *
-preprocess_targetlist(PlannerInfo *root)
+preprocess_targetlist(PlannerInfo *root, List *tlist)
 {
 	Query	   *parse = root->parse;
 	int			result_relation = parse->resultRelation;
 	List	   *range_table = parse->rtable;
 	CmdType		command_type = parse->commandType;
-	List	   *tlist;
 	ListCell   *lc;
 
 	/*
@@ -87,16 +81,6 @@ preprocess_targetlist(PlannerInfo *root)
 			elog(ERROR, "subquery cannot be result relation");
 	}
 
-	/*
-	 * For UPDATE/DELETE, add a necessary junk column needed to allow the
-	 * executor to identify the physical locations of the rows to be updated
-	 * or deleted.
-	 */
-	if (command_type == CMD_UPDATE || command_type == CMD_DELETE)
-		rewrite_targetlist(parse, result_relation, range_table);
-
-	tlist = parse->targetList;
-
 	/*
 	 * for heap_form_tuple to work, the targetlist must match the exact order
 	 * of the attributes. We also need to fill in any missing attributes. -ay
@@ -407,96 +391,6 @@ expand_targetlist(List *tlist, int command_type,
 	return new_tlist;
 }
 
-/*
- * rewrite_targetlist - rewrite UPDATE/DELETE targetlist as needed
- *
- * This function adds a "junk" TLE that is needed to allow the executor to
- * find the original row for the update or delete.  When the target relation
- * is a regular table, the junk TLE emits the ctid attribute of the original
- * row.  When the target relation is a foreign table, we let the FDW decide
- * what to add.
- */
-static void
-rewrite_targetlist(Query *parse, Index result_relation, List *range_table)
-{
-	Var		   *var = NULL;
-	const char *attrname;
-	TargetEntry *tle;
-	RangeTblEntry *target_rte;
-	Relation target_relation;
-
-	target_rte = rt_fetch(result_relation, range_table);
-
-	/*
-	 * We assume that the relation was already locked by the rewriter, so
-	 * we need no lock here.
-	 */
-	target_relation = heap_open(target_rte->relid, NoLock);
-
-	if (target_relation->rd_rel->relkind == RELKIND_RELATION ||
-		target_relation->rd_rel->relkind == RELKIND_MATVIEW ||
-		target_relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
-	{
-		/*
-		 * Emit CTID so that executor can find the row to update or delete.
-		 */
-		var = makeVar(result_relation,
-					  SelfItemPointerAttributeNumber,
-					  TIDOID,
-					  -1,
-					  InvalidOid,
-					  0);
-
-		attrname = "ctid";
-	}
-	else if (target_relation->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
-	{
-		/*
-		 * Let the foreign table's FDW add whatever junk TLEs it wants.
-		 */
-		FdwRoutine *fdwroutine;
-
-		fdwroutine = GetFdwRoutineForRelation(target_relation, false);
-
-		if (fdwroutine->AddForeignUpdateTargets != NULL)
-			fdwroutine->AddForeignUpdateTargets(parse, target_rte,
-												target_relation);
-
-		/*
-		 * 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 &&
-			((parse->commandType == CMD_UPDATE &&
-			  (target_relation->trigdesc->trig_update_after_row ||
-			   target_relation->trigdesc->trig_update_before_row)) ||
-			 (parse->commandType == CMD_DELETE &&
-			  (target_relation->trigdesc->trig_delete_after_row ||
-			   target_relation->trigdesc->trig_delete_before_row))))
-		{
-			var = makeWholeRowVar(target_rte,
-								  result_relation,
-								  0,
-								  false);
-
-			attrname = "wholerow";
-		}
-	}
-
-	if (var != NULL)
-	{
-		tle = makeTargetEntry((Expr *) var,
-							  list_length(parse->targetList) + 1,
-							  pstrdup(attrname),
-							  true);
-
-		parse->targetList = lappend(parse->targetList, tle);
-	}
-
-	heap_close(target_relation, NoLock);
-}
-
 
 /*
  * Locate PlanRowMark for given RT index, or return NULL if none
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
diff --git a/src/include/optimizer/prep.h b/src/include/optimizer/prep.h

Original file line number	Diff line number	Diff line change
`@@ -1687,7 +1687,6 @@ ExecModifyTable(PlanState *pstate)`
`1687`	`1687`	`EvalPlanQualSetSlot(&node->mt_epqstate, planSlot);`
`1688`	`1688`	`slot = planSlot;`
`1689`	`1689`
`1690`		`- tupleid = NULL;`
`1691`	`1690`	`oldtuple = NULL;`
`1692`	`1691`	`if (junkfilter != NULL)`
`1693`	`1692`	`{`