Rethink representation of PathTargets.

In commit 19a5411 I did not make PathTarget a subtype of Node,
and embedded a RelOptInfo's reltarget directly into it rather than having
a separately-allocated Node.  In hindsight that was misguided
micro-optimization, enabled by the fact that at that point we didn't have
any Paths with custom PathTargets.  Now that PathTarget processing has
been fleshed out some more, it's easier to see that it's better to have
PathTarget as an indepedent Node type, even if it does cost us one more
palloc to create a RelOptInfo.  So change it while we still can.

This commit just changes the representation, without doing anything more
interesting than that.

Author: tglsfdc

contrib/file_fdw/file_fdw.c

@@ -821,7 +821,7 @@ check_selective_binary_conversion(RelOptInfo *baserel,
 	}
 
 	/* Collect all the attributes needed for joins or final output. */
-	pull_varattnos((Node *) baserel->reltarget.exprs, baserel->relid,
+	pull_varattnos((Node *) baserel->reltarget->exprs, baserel->relid,
 				   &attrs_used);
 
 	/* Add all the attributes used by restriction clauses. */
@@ -953,7 +953,7 @@ estimate_size(PlannerInfo *root, RelOptInfo *baserel,
 		 */
 		int			tuple_width;
 
-		tuple_width = MAXALIGN(baserel->reltarget.width) +
+		tuple_width = MAXALIGN(baserel->reltarget->width) +
 			MAXALIGN(SizeofHeapTupleHeader);
 		ntuples = clamp_row_est((double) stat_buf.st_size /
 								(double) tuple_width);

contrib/postgres_fdw/deparse.c

@@ -728,10 +728,10 @@ build_tlist_to_deparse(RelOptInfo *foreignrel)
 	PgFdwRelationInfo *fpinfo = (PgFdwRelationInfo *) foreignrel->fdw_private;
 
 	/*
-	 * We require columns specified in foreignrel->reltarget.exprs and those
+	 * We require columns specified in foreignrel->reltarget->exprs and those
 	 * required for evaluating the local conditions.
 	 */
-	tlist = add_to_flat_tlist(tlist, foreignrel->reltarget.exprs);
+	tlist = add_to_flat_tlist(tlist, foreignrel->reltarget->exprs);
 	tlist = add_to_flat_tlist(tlist,
 							  pull_var_clause((Node *) fpinfo->local_conds,
 											  PVC_RECURSE_PLACEHOLDERS));

contrib/postgres_fdw/postgres_fdw.c

@@ -485,7 +485,7 @@ postgresGetForeignRelSize(PlannerInfo *root,
 	 * columns used in them.  Doesn't seem worth detecting that case though.)
 	 */
 	fpinfo->attrs_used = NULL;
-	pull_varattnos((Node *) baserel->reltarget.exprs, baserel->relid,
+	pull_varattnos((Node *) baserel->reltarget->exprs, baserel->relid,
 				   &fpinfo->attrs_used);
 	foreach(lc, fpinfo->local_conds)
 	{
@@ -536,7 +536,7 @@ postgresGetForeignRelSize(PlannerInfo *root,
 
 		/* Report estimated baserel size to planner. */
 		baserel->rows = fpinfo->rows;
-		baserel->reltarget.width = fpinfo->width;
+		baserel->reltarget->width = fpinfo->width;
 	}
 	else
 	{
@@ -553,7 +553,7 @@ postgresGetForeignRelSize(PlannerInfo *root,
 		{
 			baserel->pages = 10;
 			baserel->tuples =
-				(10 * BLCKSZ) / (baserel->reltarget.width +
+				(10 * BLCKSZ) / (baserel->reltarget->width +
 								 MAXALIGN(SizeofHeapTupleHeader));
 		}
 
@@ -2168,7 +2168,7 @@ estimate_path_cost_size(PlannerInfo *root,
 		 * between foreign relations.
 		 */
 		rows = foreignrel->rows;
-		width = foreignrel->reltarget.width;
+		width = foreignrel->reltarget->width;
 
 		/* Back into an estimate of the number of retrieved rows. */
 		retrieved_rows = clamp_row_est(rows / fpinfo->local_conds_sel);
@@ -3694,7 +3694,7 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
 							&width, &startup_cost, &total_cost);
 	/* Now update this information in the joinrel */
 	joinrel->rows = rows;
-	joinrel->reltarget.width = width;
+	joinrel->reltarget->width = width;
 	fpinfo->rows = rows;
 	fpinfo->width = width;
 	fpinfo->startup_cost = startup_cost;

doc/src/sgml/fdwhandler.sgml

@@ -1173,7 +1173,7 @@ GetForeignServerByName(const char *name, bool missing_ok);
      it contains restriction quals (<literal>WHERE</> clauses) that should be
      used to filter the rows to be fetched.  (The FDW itself is not required
      to enforce these quals, as the core executor can check them instead.)
-     <literal>baserel-&gt;reltarget.exprs</> can be used to determine which
+     <literal>baserel-&gt;reltarget-&gt;exprs</> can be used to determine which
      columns need to be fetched; but note that it only lists columns that
      have to be emitted by the <structname>ForeignScan</> plan node, not
      columns that are used in qual evaluation but not output by the query.

src/backend/nodes/outfuncs.c

@@ -1598,22 +1598,8 @@ _outPathInfo(StringInfo str, const Path *node)
 	WRITE_ENUM_FIELD(pathtype, NodeTag);
 	appendStringInfoString(str, " :parent_relids ");
 	_outBitmapset(str, node->parent->relids);
-	if (node->pathtarget != &(node->parent->reltarget))
-	{
-		WRITE_NODE_FIELD(pathtarget->exprs);
-		if (node->pathtarget->sortgrouprefs)
-		{
-			int			i;
-
-			appendStringInfoString(str, " :pathtarget->sortgrouprefs");
-			for (i = 0; i < list_length(node->pathtarget->exprs); i++)
-				appendStringInfo(str, " %u",
-								 node->pathtarget->sortgrouprefs[i]);
-		}
-		WRITE_FLOAT_FIELD(pathtarget->cost.startup, "%.2f");
-		WRITE_FLOAT_FIELD(pathtarget->cost.per_tuple, "%.2f");
-		WRITE_INT_FIELD(pathtarget->width);
-	}
+	if (node->pathtarget != node->parent->reltarget)
+		WRITE_NODE_FIELD(pathtarget);
 	appendStringInfoString(str, " :required_outer ");
 	if (node->param_info)
 		_outBitmapset(str, node->param_info->ppi_req_outer);
@@ -2094,11 +2080,7 @@ _outRelOptInfo(StringInfo str, const RelOptInfo *node)
 	WRITE_BOOL_FIELD(consider_startup);
 	WRITE_BOOL_FIELD(consider_param_startup);
 	WRITE_BOOL_FIELD(consider_parallel);
-	WRITE_NODE_FIELD(reltarget.exprs);
-	/* reltarget.sortgrouprefs is never interesting, at present anyway */
-	WRITE_FLOAT_FIELD(reltarget.cost.startup, "%.2f");
-	WRITE_FLOAT_FIELD(reltarget.cost.per_tuple, "%.2f");
-	WRITE_INT_FIELD(reltarget.width);
+	WRITE_NODE_FIELD(reltarget);
 	WRITE_NODE_FIELD(pathlist);
 	WRITE_NODE_FIELD(ppilist);
 	WRITE_NODE_FIELD(partial_pathlist);
@@ -2201,6 +2183,25 @@ _outPathKey(StringInfo str, const PathKey *node)
 	WRITE_BOOL_FIELD(pk_nulls_first);
 }
 
+static void
+_outPathTarget(StringInfo str, const PathTarget *node)
+{
+	WRITE_NODE_TYPE("PATHTARGET");
+
+	WRITE_NODE_FIELD(exprs);
+	if (node->sortgrouprefs)
+	{
+		int			i;
+
+		appendStringInfoString(str, " :sortgrouprefs");
+		for (i = 0; i < list_length(node->exprs); i++)
+			appendStringInfo(str, " %u", node->sortgrouprefs[i]);
+	}
+	WRITE_FLOAT_FIELD(cost.startup, "%.2f");
+	WRITE_FLOAT_FIELD(cost.per_tuple, "%.2f");
+	WRITE_INT_FIELD(width);
+}
+
 static void
 _outParamPathInfo(StringInfo str, const ParamPathInfo *node)
 {
@@ -3612,6 +3613,9 @@ _outNode(StringInfo str, const void *obj)
 			case T_PathKey:
 				_outPathKey(str, obj);
 				break;
+			case T_PathTarget:
+				_outPathTarget(str, obj);
+				break;
 			case T_ParamPathInfo:
 				_outParamPathInfo(str, obj);
 				break;

src/backend/optimizer/path/allpaths.c

@@ -936,7 +936,7 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
 		/*
 		 * CE failed, so finish copying/modifying targetlist and join quals.
 		 *
-		 * Note: the resulting childrel->reltarget.exprs may contain arbitrary
+		 * NB: the resulting childrel->reltarget->exprs may contain arbitrary
 		 * expressions, which otherwise would not occur in a rel's targetlist.
 		 * Code that might be looking at an appendrel child must cope with
 		 * such.  (Normally, a rel's targetlist would only include Vars and
@@ -947,9 +947,9 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
 			adjust_appendrel_attrs(root,
 								   (Node *) rel->joininfo,
 								   appinfo);
-		childrel->reltarget.exprs = (List *)
+		childrel->reltarget->exprs = (List *)
 			adjust_appendrel_attrs(root,
-								   (Node *) rel->reltarget.exprs,
+								   (Node *) rel->reltarget->exprs,
 								   appinfo);
 
 		/*
@@ -994,7 +994,7 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
 		Assert(childrel->rows > 0);
 
 		parent_rows += childrel->rows;
-		parent_size += childrel->reltarget.width * childrel->rows;
+		parent_size += childrel->reltarget->width * childrel->rows;
 
 		/*
 		 * Accumulate per-column estimates too.  We need not do anything for
@@ -1004,8 +1004,8 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
 		 *
 		 * By construction, child's targetlist is 1-to-1 with parent's.
 		 */
-		forboth(parentvars, rel->reltarget.exprs,
-				childvars, childrel->reltarget.exprs)
+		forboth(parentvars, rel->reltarget->exprs,
+				childvars, childrel->reltarget->exprs)
 		{
 			Var		   *parentvar = (Var *) lfirst(parentvars);
 			Node	   *childvar = (Node *) lfirst(childvars);
@@ -1040,7 +1040,7 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
 
 		Assert(parent_rows > 0);
 		rel->rows = parent_rows;
-		rel->reltarget.width = rint(parent_size / parent_rows);
+		rel->reltarget->width = rint(parent_size / parent_rows);
 		for (i = 0; i < nattrs; i++)
 			rel->attr_widths[i] = rint(parent_attrsizes[i] / parent_rows);
 
@@ -1515,7 +1515,7 @@ set_dummy_rel_pathlist(RelOptInfo *rel)
 {
 	/* Set dummy size estimates --- we leave attr_widths[] as zeroes */
 	rel->rows = 0;
-	rel->reltarget.width = 0;
+	rel->reltarget->width = 0;
 
 	/* Discard any pre-existing paths; no further need for them */
 	rel->pathlist = NIL;
@@ -1771,7 +1771,7 @@ set_function_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
 		 * not reference the ordinality column, or at least not in any way
 		 * that would be interesting for sorting.
 		 */
-		foreach(lc, rel->reltarget.exprs)
+		foreach(lc, rel->reltarget->exprs)
 		{
 			Var		   *node = (Var *) lfirst(lc);
 
@@ -2717,7 +2717,7 @@ remove_unused_subquery_outputs(Query *subquery, RelOptInfo *rel)
 	 * isn't computed for inheritance child rels, cf set_append_rel_size().
 	 * (XXX might be worth changing that sometime.)
 	 */
-	pull_varattnos((Node *) rel->reltarget.exprs, rel->relid, &attrs_used);
+	pull_varattnos((Node *) rel->reltarget->exprs, rel->relid, &attrs_used);
 
 	/* Add all the attributes used by un-pushed-down restriction clauses. */
 	foreach(lc, rel->baserestrictinfo)
@@ -3028,7 +3028,7 @@ debug_print_rel(PlannerInfo *root, RelOptInfo *rel)
 
 	printf("RELOPTINFO (");
 	print_relids(rel->relids);
-	printf("): rows=%.0f width=%d\n", rel->rows, rel->reltarget.width);
+	printf("): rows=%.0f width=%d\n", rel->rows, rel->reltarget->width);
 
 	if (rel->baserestrictinfo)
 	{

src/backend/optimizer/path/costsize.c

@@ -4218,7 +4218,7 @@ set_foreign_size_estimates(PlannerInfo *root, RelOptInfo *rel)
  * that have to be calculated at this relation.  This is the amount of data
  * we'd need to pass upwards in case of a sort, hash, etc.
  *
- * This function also sets reltarget.cost, so it's a bit misnamed now.
+ * This function also sets reltarget->cost, so it's a bit misnamed now.
  *
  * NB: this works best on plain relations because it prefers to look at
  * real Vars.  For subqueries, set_subquery_size_estimates will already have
@@ -4239,10 +4239,10 @@ set_rel_width(PlannerInfo *root, RelOptInfo *rel)
 	ListCell   *lc;
 
 	/* Vars are assumed to have cost zero, but other exprs do not */
-	rel->reltarget.cost.startup = 0;
-	rel->reltarget.cost.per_tuple = 0;
+	rel->reltarget->cost.startup = 0;
+	rel->reltarget->cost.per_tuple = 0;
 
-	foreach(lc, rel->reltarget.exprs)
+	foreach(lc, rel->reltarget->exprs)
 	{
 		Node	   *node = (Node *) lfirst(lc);
 
@@ -4309,16 +4309,16 @@ set_rel_width(PlannerInfo *root, RelOptInfo *rel)
 		{
 			/*
 			 * We will need to evaluate the PHV's contained expression while
-			 * scanning this rel, so be sure to include it in reltarget.cost.
+			 * scanning this rel, so be sure to include it in reltarget->cost.
 			 */
 			PlaceHolderVar *phv = (PlaceHolderVar *) node;
 			PlaceHolderInfo *phinfo = find_placeholder_info(root, phv, false);
 			QualCost	cost;
 
 			tuple_width += phinfo->ph_width;
 			cost_qual_eval_node(&cost, (Node *) phv->phexpr, root);
-			rel->reltarget.cost.startup += cost.startup;
-			rel->reltarget.cost.per_tuple += cost.per_tuple;
+			rel->reltarget->cost.startup += cost.startup;
+			rel->reltarget->cost.per_tuple += cost.per_tuple;
 		}
 		else
 		{
@@ -4335,8 +4335,8 @@ set_rel_width(PlannerInfo *root, RelOptInfo *rel)
 			tuple_width += item_width;
 			/* Not entirely clear if we need to account for cost, but do so */
 			cost_qual_eval_node(&cost, node, root);
-			rel->reltarget.cost.startup += cost.startup;
-			rel->reltarget.cost.per_tuple += cost.per_tuple;
+			rel->reltarget->cost.startup += cost.startup;
+			rel->reltarget->cost.per_tuple += cost.per_tuple;
 		}
 	}
 
@@ -4373,7 +4373,7 @@ set_rel_width(PlannerInfo *root, RelOptInfo *rel)
 	}
 
 	Assert(tuple_width >= 0);
-	rel->reltarget.width = tuple_width;
+	rel->reltarget->width = tuple_width;
 }
 
 /*

src/backend/optimizer/path/indxpath.c

@@ -1550,7 +1550,7 @@ bitmap_scan_cost_est(PlannerInfo *root, RelOptInfo *rel, Path *ipath)
 	bpath.path.type = T_BitmapHeapPath;
 	bpath.path.pathtype = T_BitmapHeapScan;
 	bpath.path.parent = rel;
-	bpath.path.pathtarget = &(rel->reltarget);
+	bpath.path.pathtarget = rel->reltarget;
 	bpath.path.param_info = get_baserel_parampathinfo(root, rel,
 													  required_outer);
 	bpath.path.pathkeys = NIL;
@@ -1579,7 +1579,7 @@ bitmap_and_cost_est(PlannerInfo *root, RelOptInfo *rel, List *paths)
 	apath.path.type = T_BitmapAndPath;
 	apath.path.pathtype = T_BitmapAnd;
 	apath.path.parent = rel;
-	apath.path.pathtarget = &(rel->reltarget);
+	apath.path.pathtarget = rel->reltarget;
 	apath.path.param_info = NULL;		/* not used in bitmap trees */
 	apath.path.pathkeys = NIL;
 	apath.bitmapquals = paths;
@@ -1592,7 +1592,7 @@ bitmap_and_cost_est(PlannerInfo *root, RelOptInfo *rel, List *paths)
 	bpath.path.type = T_BitmapHeapPath;
 	bpath.path.pathtype = T_BitmapHeapScan;
 	bpath.path.parent = rel;
-	bpath.path.pathtarget = &(rel->reltarget);
+	bpath.path.pathtarget = rel->reltarget;
 	bpath.path.param_info = get_baserel_parampathinfo(root, rel,
 													  required_outer);
 	bpath.path.pathkeys = NIL;
@@ -1815,7 +1815,7 @@ check_index_only(RelOptInfo *rel, IndexOptInfo *index)
 	 * look at rel's targetlist, not the attr_needed data, because attr_needed
 	 * isn't computed for inheritance child rels.
 	 */
-	pull_varattnos((Node *) rel->reltarget.exprs, rel->relid, &attrs_used);
+	pull_varattnos((Node *) rel->reltarget->exprs, rel->relid, &attrs_used);
 
 	/* Add all the attributes used by restriction clauses. */
 	foreach(lc, rel->baserestrictinfo)

src/backend/optimizer/plan/createplan.c

@@ -3279,7 +3279,7 @@ create_foreignscan_plan(PlannerInfo *root, ForeignPath *best_path,
 		 * Note: we must look at rel's targetlist, not the attr_needed data,
 		 * because attr_needed isn't computed for inheritance child rels.
 		 */
-		pull_varattnos((Node *) rel->reltarget.exprs, scan_relid, &attrs_used);
+		pull_varattnos((Node *) rel->reltarget->exprs, scan_relid, &attrs_used);
 
 		/* Add all the attributes used by restriction clauses. */
 		foreach(lc, rel->baserestrictinfo)

src/backend/optimizer/plan/initsplan.c

@@ -215,8 +215,8 @@ add_vars_to_targetlist(PlannerInfo *root, List *vars,
 			{
 				/* Variable not yet requested, so add to rel's targetlist */
 				/* XXX is copyObject necessary here? */
-				rel->reltarget.exprs = lappend(rel->reltarget.exprs,
-											   copyObject(var));
+				rel->reltarget->exprs = lappend(rel->reltarget->exprs,
+												copyObject(var));
 				/* reltarget cost and width will be computed later */
 			}
 			rel->attr_needed[attno] = bms_add_members(rel->attr_needed[attno],