From a57aa430b613250270cf5cc78a34a3454e61acda 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 cfa68084179..67f512d1240 100644 --- a/doc/src/sgml/fdwhandler.sgml +++ b/doc/src/sgml/fdwhandler.sgml @@ -429,11 +429,14 @@ AddForeignUpdateTargets (Query *parsetree, wholerow, or wholerowN, 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 eval_const_expressions + before adding them to the targetlist. - 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. parsetree is the parse tree for the UPDATE or DELETE command, while target_rte and diff --git a/doc/src/sgml/rules.sgml b/doc/src/sgml/rules.sgml index bcbc1703355..6344da95855 100644 --- a/doc/src/sgml/rules.sgml +++ b/doc/src/sgml/rules.sgml @@ -167,12 +167,12 @@ DELETE 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 CTID entry to the empty target list, to allow the executor to find the row to be deleted. (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 .) + table. If it is a view, a whole-row variable is added instead, by + the rule system, as described in .) @@ -194,7 +194,7 @@ column = expression 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 DELETE, - the rule system adds a CTID or whole-row variable so that + a CTID or whole-row variable is added so that the executor can identify the old row to be updated. -- cgit v1.2.3