Fix EXPLAIN output for cases where parent table is excluded by constraints.

tglsfdc · tglsfdc · commit abe45a9b315d · 2015-02-17T18:04:11.000-05:00
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
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);

Original file line number	Diff line number	Diff line change
`@@ -754,6 +754,8 @@ set_plan_refs(PlannerInfo root, Plan plan, int rtoffset)`
`754`	`754`	`splan->plan.targetlist = copyObject(linitial(newRL));`
`755`	`755`	`}`
`756`	`756`
	`757`	`+ splan->nominalRelation += rtoffset;`
	`758`	`+`
`757`	`759`	`foreach(l, splan->resultRelations)`
`758`	`760`	`{`
`759`	`761`	`lfirst_int(l) += rtoffset;`