diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 4339bbf..f90a6cf 100644 *** a/contrib/postgres_fdw/expected/postgres_fdw.out --- b/contrib/postgres_fdw/expected/postgres_fdw.out *************** update bar set f2 = f2 + 100 returning * *** 7022,7027 **** --- 7022,7086 ---- 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 index ddfec79..8e1bd89 100644 *** a/contrib/postgres_fdw/sql/postgres_fdw.sql --- b/contrib/postgres_fdw/sql/postgres_fdw.sql *************** explain (verbose, costs off) *** 1656,1661 **** --- 1656,1681 ---- 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 index a2f8137..e610fa0 100644 *** a/doc/src/sgml/fdwhandler.sgml --- b/doc/src/sgml/fdwhandler.sgml *************** AddForeignUpdateTargets(Query *parsetree *** 428,438 **** Avoid using names matching ctidN, wholerow, or wholerowN, as the core system can ! generate junk columns of these names. ! 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. parsetree is the parse tree for the UPDATE or --- 428,440 ---- Avoid using names matching ctidN, wholerow, or wholerowN, 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 eval_const_expressions if necessary. ! Although this function is called in the planner, the information available is a bit different from that available to the planning routines. parsetree is the parse tree for the UPDATE or diff --git a/doc/src/sgml/rules.sgml b/doc/src/sgml/rules.sgml index 2074fcc..3372b1a 100644 *** a/doc/src/sgml/rules.sgml --- b/doc/src/sgml/rules.sgml *************** *** 167,178 **** DELETE commands don't need a normal target list ! because they don't produce any result. Instead, the rule system 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 .) --- 167,178 ---- DELETE commands don't need a normal target list ! 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, by ! the rule system, as described in .) *************** *** 194,200 **** 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 the executor can identify the old row to be updated. --- 194,200 ---- 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, ! a CTID or whole-row variable is added so that the executor can identify the old row to be updated. diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 503b89f..1f5ef02 100644 *** a/src/backend/executor/nodeModifyTable.c --- b/src/backend/executor/nodeModifyTable.c *************** ExecModifyTable(PlanState *pstate) *** 1699,1704 **** --- 1699,1705 ---- 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 index f6b8bbf..ef2eaea 100644 *** a/src/backend/optimizer/plan/planner.c --- b/src/backend/optimizer/plan/planner.c *************** grouping_planner(PlannerInfo *root, bool *** 1555,1561 **** double tuple_fraction) { Query *parse = root->parse; ! List *tlist = parse->targetList; int64 offset_est = 0; int64 count_est = 0; double limit_tuples = -1.0; --- 1555,1561 ---- double tuple_fraction) { Query *parse = root->parse; ! List *tlist; int64 offset_est = 0; int64 count_est = 0; double limit_tuples = -1.0; *************** grouping_planner(PlannerInfo *root, bool *** 1685,1697 **** } /* Preprocess targetlist */ ! tlist = preprocess_targetlist(root, tlist); ! ! if (parse->onConflict) ! parse->onConflict->onConflictSet = ! preprocess_onconflict_targetlist(parse->onConflict->onConflictSet, ! parse->resultRelation, ! parse->rtable); /* * We are now done hacking up the query's targetlist. Most of the --- 1685,1691 ---- } /* Preprocess targetlist */ ! tlist = preprocess_targetlist(root); /* * We are now done hacking up the query's targetlist. Most of the diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c index d7db32e..44f3ae8 100644 *** a/src/backend/optimizer/prep/preptlist.c --- b/src/backend/optimizer/prep/preptlist.c *************** *** 4,23 **** * 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 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 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 --- 4,25 ---- * 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. * ! * 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 rewriteTargetListIU 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 relation type and column list of the ! * child, which might be of a different type, or 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 *************** *** 47,57 **** #include "optimizer/var.h" #include "parser/parsetree.h" #include "parser/parse_coerce.h" #include "utils/rel.h" static List *expand_targetlist(List *tlist, int command_type, ! Index result_relation, List *range_table); /* --- 49,60 ---- #include "optimizer/var.h" #include "parser/parsetree.h" #include "parser/parse_coerce.h" + #include "rewrite/rewriteHandler.h" #include "utils/rel.h" static List *expand_targetlist(List *tlist, int command_type, ! Index result_relation, Relation rel); /* *************** static List *expand_targetlist(List *tli *** 59,94 **** * Driver for preprocessing the parse tree targetlist. * * Returns the new targetlist. */ List * ! 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; ListCell *lc; /* ! * Sanity check: if there is a result relation, it'd better be a real ! * relation not a subquery. Else parser or rewriter messed up. */ if (result_relation) { ! RangeTblEntry *rte = rt_fetch(result_relation, range_table); ! if (rte->subquery != NULL || rte->relid == InvalidOid) ! elog(ERROR, "subquery cannot be result relation"); } /* * 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 * 10/94 */ if (command_type == CMD_INSERT || command_type == CMD_UPDATE) tlist = expand_targetlist(tlist, command_type, ! result_relation, range_table); /* * Add necessary junk columns for rowmarked rels. These values are needed --- 62,120 ---- * Driver for preprocessing the parse tree targetlist. * * Returns the new targetlist. + * + * As a side effect, if there's an ON CONFLICT UPDATE clause, its targetlist + * is also preprocessed (and updated in-place). */ List * ! preprocess_targetlist(PlannerInfo *root) { Query *parse = root->parse; int result_relation = parse->resultRelation; List *range_table = parse->rtable; CmdType command_type = parse->commandType; + RangeTblEntry *target_rte = NULL; + Relation target_relation = NULL; + List *tlist; ListCell *lc; /* ! * If there is a result relation, open it so we can look for missing ! * columns and so on. We assume that previous code already acquired at ! * least AccessShareLock on the relation, so we need no lock here. */ if (result_relation) { ! target_rte = rt_fetch(result_relation, range_table); ! /* ! * Sanity check: it'd better be a real relation not, say, a subquery. ! * Else parser or rewriter messed up. ! */ ! if (target_rte->rtekind != RTE_RELATION) ! elog(ERROR, "result relation must be a regular relation"); ! ! target_relation = heap_open(target_rte->relid, NoLock); } + else + Assert(command_type == CMD_SELECT); + + /* + * For UPDATE/DELETE, add any junk column(s) needed to allow the executor + * to identify the rows to be updated or deleted. + */ + if (command_type == CMD_UPDATE || command_type == CMD_DELETE) + rewriteTargetListUD(parse, target_rte, target_relation); /* * 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 * 10/94 */ + tlist = parse->targetList; if (command_type == CMD_INSERT || command_type == CMD_UPDATE) tlist = expand_targetlist(tlist, command_type, ! result_relation, target_relation); /* * Add necessary junk columns for rowmarked rels. These values are needed *************** preprocess_targetlist(PlannerInfo *root, *** 193,211 **** list_free(vars); } ! return tlist; ! } ! /* ! * preprocess_onconflict_targetlist ! * Process ON CONFLICT SET targetlist. ! * ! * Returns the new targetlist. ! */ ! List * ! preprocess_onconflict_targetlist(List *tlist, int result_relation, List *range_table) ! { ! return expand_targetlist(tlist, CMD_UPDATE, result_relation, range_table); } --- 219,239 ---- list_free(vars); } ! /* ! * If there's an ON CONFLICT UPDATE clause, preprocess its targetlist too ! * while we have the relation open. ! */ ! if (parse->onConflict) ! parse->onConflict->onConflictSet = ! expand_targetlist(parse->onConflict->onConflictSet, ! CMD_UPDATE, ! result_relation, ! target_relation); ! if (target_relation) ! heap_close(target_relation, NoLock); ! ! return tlist; } *************** preprocess_onconflict_targetlist(List *t *** 223,233 **** */ static List * expand_targetlist(List *tlist, int command_type, ! Index result_relation, List *range_table) { List *new_tlist = NIL; ListCell *tlist_item; - Relation rel; int attrno, numattrs; --- 251,260 ---- */ static List * expand_targetlist(List *tlist, int command_type, ! Index result_relation, Relation rel) { List *new_tlist = NIL; ListCell *tlist_item; int attrno, numattrs; *************** expand_targetlist(List *tlist, int comma *** 238,249 **** * order; but we have to insert TLEs for any missing attributes. * * Scan the tuple description in the relation's relcache entry to make ! * sure we have all the user attributes in the right order. We assume ! * that the rewriter already acquired at least AccessShareLock on the ! * relation, so we need no lock here. */ - rel = heap_open(getrelid(result_relation, range_table), NoLock); - numattrs = RelationGetNumberOfAttributes(rel); for (attrno = 1; attrno <= numattrs; attrno++) --- 265,272 ---- * order; but we have to insert TLEs for any missing attributes. * * Scan the tuple description in the relation's relcache entry to make ! * sure we have all the user attributes in the right order. */ numattrs = RelationGetNumberOfAttributes(rel); for (attrno = 1; attrno <= numattrs; attrno++) *************** expand_targetlist(List *tlist, int comma *** 386,393 **** tlist_item = lnext(tlist_item); } - heap_close(rel, NoLock); - return new_tlist; } --- 409,414 ---- diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index c2bc3ad..65f1b8e 100644 *** a/src/backend/rewrite/rewriteHandler.c --- b/src/backend/rewrite/rewriteHandler.c *************** static TargetEntry *process_matched_tle( *** 72,79 **** 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 ---- *************** rewriteValuesRTE(RangeTblEntry *rte, Rel *** 1293,1306 **** * 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 view, there is no ctid, so we instead ! * emit a whole-row Var that will contain the "old" values of the view row. ! * If it's a foreign table, we let the FDW decide what to add. * ! * 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) { --- 1291,1305 ---- * 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. * ! * We used to do this during RewriteQuery(), but now that inheritance trees ! * can contain a mix of regular and foreign tables, we must postpone it till ! * planning, after the inheritance tree has been expanded. In that way we ! * can do the right thing for each child table. */ ! void rewriteTargetListUD(Query *parsetree, RangeTblEntry *target_rte, Relation target_relation) { *************** rewriteTargetListUD(Query *parsetree, Ra *** 1358,1376 **** attrname = "wholerow"; } } - else - { - /* - * Emit whole-row Var so that executor will have the "old" view row to - * pass to the INSTEAD OF trigger. - */ - var = makeWholeRowVar(target_rte, - parsetree->resultRelation, - 0, - false); - - attrname = "wholerow"; - } if (var != NULL) { --- 1357,1362 ---- *************** ApplyRetrieveRule(Query *parsetree, *** 1497,1502 **** --- 1483,1490 ---- parsetree->commandType == CMD_DELETE) { RangeTblEntry *newrte; + Var *var; + TargetEntry *tle; rte = rt_fetch(rt_index, parsetree->rtable); newrte = copyObject(rte); *************** ApplyRetrieveRule(Query *parsetree, *** 1527,1532 **** --- 1515,1533 ---- ChangeVarNodes((Node *) parsetree->returningList, rt_index, parsetree->resultRelation, 0); + /* + * To allow the executor to find the original view row to pass to + * the INSTEAD OF trigger, we add a whole-row reference to the + * original RTE to the query's targetlist. + */ + var = makeWholeRowVar(rte, rt_index, 0, false); + tle = makeTargetEntry((Expr *) var, + list_length(parsetree->targetList) + 1, + pstrdup("wholerow"), + true); + + parsetree->targetList = lappend(parsetree->targetList, tle); + /* Now, continue with expanding the original view RTE */ } else *************** rewriteTargetView(Query *parsetree, Rela *** 2967,2992 **** view_rte->securityQuals = NIL; /* - * For UPDATE/DELETE, rewriteTargetListUD will have added a wholerow junk - * TLE for the view to the end of the targetlist, which we no longer need. - * Remove it to avoid unnecessary work when we process the targetlist. - * Note that when we recurse through rewriteQuery a new junk TLE will be - * added to allow the executor to find the proper row in the new target - * relation. (So, if we failed to do this, we might have multiple junk - * TLEs with the same name, which would be disastrous.) - */ - if (parsetree->commandType != CMD_INSERT) - { - TargetEntry *tle = (TargetEntry *) llast(parsetree->targetList); - - Assert(tle->resjunk); - Assert(IsA(tle->expr, Var) && - ((Var *) tle->expr)->varno == parsetree->resultRelation && - ((Var *) tle->expr)->varattno == 0); - parsetree->targetList = list_delete_ptr(parsetree->targetList, tle); - } - - /* * Now update all Vars in the outer query that reference the view to * reference the appropriate column of the base relation instead. */ --- 2968,2973 ---- *************** RewriteQuery(Query *parsetree, List *rew *** 3347,3357 **** parsetree->override, rt_entry_relation, parsetree->resultRelation, NULL); - rewriteTargetListUD(parsetree, rt_entry, rt_entry_relation); } else if (event == CMD_DELETE) { ! rewriteTargetListUD(parsetree, rt_entry, rt_entry_relation); } else elog(ERROR, "unrecognized commandType: %d", (int) event); --- 3328,3337 ---- parsetree->override, rt_entry_relation, parsetree->resultRelation, NULL); } else if (event == CMD_DELETE) { ! /* Nothing to do here */ } else elog(ERROR, "unrecognized commandType: %d", (int) event); diff --git a/src/include/optimizer/prep.h b/src/include/optimizer/prep.h index 80fbfd6..804c9e3 100644 *** a/src/include/optimizer/prep.h --- b/src/include/optimizer/prep.h *************** extern Expr *canonicalize_qual(Expr *qua *** 38,47 **** /* * prototypes for preptlist.c */ ! extern List *preprocess_targetlist(PlannerInfo *root, List *tlist); ! ! extern List *preprocess_onconflict_targetlist(List *tlist, ! int result_relation, List *range_table); extern PlanRowMark *get_plan_rowmark(List *rowmarks, Index rtindex); --- 38,44 ---- /* * prototypes for preptlist.c */ ! extern List *preprocess_targetlist(PlannerInfo *root); extern PlanRowMark *get_plan_rowmark(List *rowmarks, Index rtindex); diff --git a/src/include/rewrite/rewriteHandler.h b/src/include/rewrite/rewriteHandler.h index 494fa29..86ae571 100644 *** a/src/include/rewrite/rewriteHandler.h --- b/src/include/rewrite/rewriteHandler.h *************** extern void AcquireRewriteLocks(Query *p *** 23,28 **** --- 23,31 ---- 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);