From 39f180fdd1e68bf6a897c6ab8733014e74f56973 Mon Sep 17 00:00:00 2001
From: Tom Lane
Date: Mon, 27 Nov 2017 17:53:56 -0500
Subject: 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
---
 doc/src/sgml/fdwhandler.sgml | 7 +++++--
 doc/src/sgml/rules.sgml      | 8 ++++----
 2 files changed, 9 insertions(+), 6 deletions(-)

(limited to 'doc/src')

diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml
index 98fda71e106..6dae0bd79b3 100644
--- a/doc/src/sgml/fdwhandler.sgml
+++ b/doc/src/sgml/fdwhandler.sgml
@@ -382,11 +382,14 @@ AddForeignUpdateTargets (Query *parsetree,
      <literal>wholerow</literal>, or
      <literal>wholerow<replaceable>N</></literal>, as the core system can
      generate junk columns of these names.
+     If the extra expressions are more complex than simple Vars, they
+     must be run through <function>eval_const_expressions</function>
+     before adding them to the targetlist.
     </para>
 
     <para>
-     This function is called in the rewriter, not the planner, so the
-     information available is a bit different from that available to the
+     Although this function is called during planning, the
+     information provided is a bit different from that available to other
      planning routines.
      <literal>parsetree</> is the parse tree for the <command>UPDATE</> or
      <command>DELETE</> command, while <literal>target_rte</> and
diff --git a/doc/src/sgml/rules.sgml b/doc/src/sgml/rules.sgml
index 1816ab59888..a55d28fe243 100644
--- 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 rule system
+        because they don't produce any result.  Instead, the planner
         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,
-        as described in <xref linkend="rules-views-update">.)
+        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">.)
     </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</>,
-        the rule system adds a <acronym>CTID</> or whole-row variable so that
+        a <acronym>CTID</> or whole-row variable is added so that
         the executor can identify the old row to be updated.
     </para>
 
-- 
cgit v1.2.3