On 2017/05/16 21:11, Ashutosh Bapat wrote:
On Tue, May 16, 2017 at 5:35 PM, Ildus Kurbangaliev
<i.kurbangal...@postgrespro.ru> wrote:
I agree. Maybe this issue should be added to Postgresql Open Items?
I think there should be some complex solution that fixes not only
triggers for foreign tables at table partitioning, but covers other
possible not working cases.
I doubt if this is an open item, since DMLs on foreign tables are
supported since 9.3 and support to add foreign tables to inheritance
was added back in 9.5.
I think this issue was introduced by the latter, so that was my fault.
One approach I came up with to fix this issue is to rewrite the
targetList entries of an inherited UPDATE/DELETE properly using
rewriteTargetListUD, when generating a plan for each child table in
inheritance_planner. Attached is a WIP patch for that. Maybe I am
missing something, though.
Best regards,
Etsuro Fujita
*** a/src/backend/executor/nodeModifyTable.c
--- b/src/backend/executor/nodeModifyTable.c
***************
*** 1469,1474 **** ExecModifyTable(ModifyTableState *node)
--- 1469,1475 ----
resultRelInfo++;
subplanstate = node->mt_plans[node->mt_whichplan];
junkfilter = resultRelInfo->ri_junkFilter;
+ tupleid = NULL;
estate->es_result_relation_info = resultRelInfo;
EvalPlanQualSetPlan(&node->mt_epqstate, subplanstate->plan,
node->mt_arowmarks[node->mt_whichplan]);
*** 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 *target_rte);
/*
***************
*** 1806,1811 **** adjust_appendrel_attrs(PlannerInfo *root, Node *node, AppendRelInfo *appinfo)
--- 1808,1826 ----
newnode->targetList =
adjust_inherited_tlist(newnode->targetList,
appinfo);
+ /* And fix UPDATE/DELETE junk tlist entries 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;
}
***************
*** 2139,2144 **** adjust_inherited_tlist(List *tlist, AppendRelInfo *context)
--- 2154,2196 ----
}
/*
+ * Rewrite the targetlist entries of an inherited UPDATE/DELETE operation
+ */
+ static void
+ rewrite_inherited_tlist(Query *parse, RangeTblEntry *target_rte)
+ {
+ ListCell *tl;
+ List *new_tlist = NIL;
+ Relation target_relation;
+
+ /*
+ * Open the target relation; we need not lock it since it was already
+ * locked by expand_inherited_rtentry().
+ */
+ target_relation = heap_open(target_rte->relid, NoLock);
+
+ /* Create a new tlist without junk tlist entries */
+ foreach(tl, parse->targetList)
+ {
+ TargetEntry *tle = (TargetEntry *) lfirst(tl);
+
+ if (tle->resjunk)
+ continue; /* don't need junk items */
+
+ new_tlist = lappend(new_tlist, tle);
+ }
+
+ /* Replace the targetList with the new one */
+ parse->targetList = new_tlist;
+
+ /* And add junk tlist entries needed for UPDATE/DELETE */
+ rewriteTargetListUD(parse, target_rte, target_relation);
+
+ /* Close the target relation, but keep the lock */
+ heap_close(target_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);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
