From abe45a9b315d7b3739f442597f570f9454bd466d Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 17 Feb 2015 18:04:11 -0500 Subject: [PATCH] Fix EXPLAIN output for cases where parent table is excluded by constraints. The previous coding in EXPLAIN always labeled a ModifyTable node with the name of the target table affected by its first child plan. When originally written, this was necessarily the parent table of the inheritance tree, so everything was unconfusing. But when we added NO INHERIT constraints, it became possible for the parent table to be deleted from the plan by constraint exclusion while still leaving child tables present. This led to the ModifyTable plan node being labeled with the first surviving child, which was deemed confusing. Fix it by retaining the parent table's RT index in a new field in ModifyTable. Etsuro Fujita, reviewed by Ashutosh Bapat and myself --- src/backend/commands/explain.c | 14 ++------------ src/backend/nodes/copyfuncs.c | 1 + src/backend/nodes/outfuncs.c | 1 + src/backend/optimizer/plan/createplan.c | 2 ++ src/backend/optimizer/plan/planner.c | 19 ++++++++++++++++++- src/backend/optimizer/plan/setrefs.c | 2 ++ src/include/nodes/plannodes.h | 3 ++- src/include/optimizer/planmain.h | 1 + 8 files changed, 29 insertions(+), 14 deletions(-) diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 7cfc9bb612..a951c55ed3 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -736,9 +736,8 @@ ExplainPreScanNode(PlanState *planstate, Bitmapset **rels_used) ((Scan *) plan)->scanrelid); break; case T_ModifyTable: - /* cf ExplainModifyTarget */ *rels_used = bms_add_member(*rels_used, - linitial_int(((ModifyTable *) plan)->resultRelations)); + ((ModifyTable *) plan)->nominalRelation); break; default: break; @@ -2192,16 +2191,7 @@ ExplainScanTarget(Scan *plan, ExplainState *es) static void ExplainModifyTarget(ModifyTable *plan, ExplainState *es) { - Index rti; - - /* - * We show the name of the first target relation. In multi-target-table - * cases this should always be the parent of the inheritance tree. - */ - Assert(plan->resultRelations != NIL); - rti = linitial_int(plan->resultRelations); - - ExplainTargetRel((Plan *) plan, rti, es); + ExplainTargetRel((Plan *) plan, plan->nominalRelation, es); } /* diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index f1a24f58af..e5b0dce477 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -175,6 +175,7 @@ _copyModifyTable(const ModifyTable *from) */ COPY_SCALAR_FIELD(operation); COPY_SCALAR_FIELD(canSetTag); + COPY_SCALAR_FIELD(nominalRelation); COPY_NODE_FIELD(resultRelations); COPY_SCALAR_FIELD(resultRelIndex); COPY_NODE_FIELD(plans); diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index dd1278bba8..8486448062 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -327,6 +327,7 @@ _outModifyTable(StringInfo str, const ModifyTable *node) WRITE_ENUM_FIELD(operation, CmdType); WRITE_BOOL_FIELD(canSetTag); + WRITE_UINT_FIELD(nominalRelation); WRITE_NODE_FIELD(resultRelations); WRITE_INT_FIELD(resultRelIndex); WRITE_NODE_FIELD(plans); diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 655be8155a..76ba1bfa8d 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -4809,6 +4809,7 @@ make_result(PlannerInfo *root, ModifyTable * make_modifytable(PlannerInfo *root, CmdType operation, bool canSetTag, + Index nominalRelation, List *resultRelations, List *subplans, List *withCheckOptionLists, List *returningLists, List *rowMarks, int epqParam) @@ -4857,6 +4858,7 @@ make_modifytable(PlannerInfo *root, node->operation = operation; node->canSetTag = canSetTag; + node->nominalRelation = nominalRelation; node->resultRelations = resultRelations; node->resultRelIndex = -1; /* will be set correctly in setrefs.c */ node->plans = subplans; diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 9cbbcfb19d..5c4884f46b 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -607,6 +607,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse, plan = (Plan *) make_modifytable(root, parse->commandType, parse->canSetTag, + parse->resultRelation, list_make1_int(parse->resultRelation), list_make1(plan), withCheckOptionLists, @@ -790,6 +791,7 @@ inheritance_planner(PlannerInfo *root) { Query *parse = root->parse; int parentRTindex = parse->resultRelation; + int nominalRelation = -1; List *final_rtable = NIL; int save_rel_array_size = 0; RelOptInfo **save_rel_array = NULL; @@ -924,6 +926,20 @@ inheritance_planner(PlannerInfo *root) */ appinfo->child_relid = subroot.parse->resultRelation; + /* + * We'll use the first child relation (even if it's excluded) as the + * nominal target relation of the ModifyTable node. Because of the + * way expand_inherited_rtentry works, this should always be the RTE + * representing the parent table in its role as a simple member of the + * inheritance set. (It would be logically cleaner to use the + * inheritance parent RTE as the nominal target; but since that RTE + * will not be otherwise referenced in the plan, doing so would give + * rise to confusing use of multiple aliases in EXPLAIN output for + * what the user will think is the "same" table.) + */ + if (nominalRelation < 0) + nominalRelation = appinfo->child_relid; + /* * If this child rel was excluded by constraint exclusion, exclude it * from the result plan. @@ -1051,6 +1067,7 @@ inheritance_planner(PlannerInfo *root) return (Plan *) make_modifytable(root, parse->commandType, parse->canSetTag, + nominalRelation, resultRelations, subplans, withCheckOptionLists, @@ -2260,7 +2277,7 @@ preprocess_rowmarks(PlannerInfo *root) newrc->markType = ROW_MARK_REFERENCE; else newrc->markType = ROW_MARK_COPY; - newrc->waitPolicy = LockWaitBlock; /* doesn't matter */ + newrc->waitPolicy = LockWaitBlock; /* doesn't matter */ newrc->isParent = false; prowmarks = lappend(prowmarks, newrc); diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c index 7703946c85..57195e5d68 100644 --- a/src/backend/optimizer/plan/setrefs.c +++ b/src/backend/optimizer/plan/setrefs.c @@ -754,6 +754,8 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset) splan->plan.targetlist = copyObject(linitial(newRL)); } + splan->nominalRelation += rtoffset; + foreach(l, splan->resultRelations) { lfirst_int(l) += rtoffset; diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index 316c9ced59..f6683f05e3 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -70,7 +70,7 @@ typedef struct PlannedStmt int nParamExec; /* number of PARAM_EXEC Params used */ - bool hasRowSecurity; /* row security applied? */ + bool hasRowSecurity; /* row security applied? */ } PlannedStmt; @@ -174,6 +174,7 @@ typedef struct ModifyTable Plan plan; CmdType operation; /* INSERT, UPDATE, or DELETE */ bool canSetTag; /* do we set the command tag/es_processed? */ + Index nominalRelation; /* Parent RT index for use of EXPLAIN */ List *resultRelations; /* integer list of RT indexes */ int resultRelIndex; /* index of first resultRel in plan's list */ List *plans; /* plan(s) producing source data */ diff --git a/src/include/optimizer/planmain.h b/src/include/optimizer/planmain.h index 082f7d7f14..fa72918d1b 100644 --- a/src/include/optimizer/planmain.h +++ b/src/include/optimizer/planmain.h @@ -82,6 +82,7 @@ extern Result *make_result(PlannerInfo *root, List *tlist, Node *resconstantqual, Plan *subplan); extern ModifyTable *make_modifytable(PlannerInfo *root, CmdType operation, bool canSetTag, + Index nominalRelation, List *resultRelations, List *subplans, List *withCheckOptionLists, List *returningLists, List *rowMarks, int epqParam);