3 files changed, 45 insertions, 33 deletions

diff --git a/src/backend/optimizer/plan/planagg.c b/src/backend/optimizer/plan/planagg.c
index 9330908cbf1..e0e357960f2 100644
--- a/src/backend/optimizer/plan/planagg.c
+++ b/src/backend/optimizer/plan/planagg.c
@@ -137,8 +137,8 @@ preprocess_minmax_aggregates(PlannerInfo *root)
 		return;
 
 	/*
-	 * Scan the tlist and HAVING qual to find all the aggregates and verify
-	 * all are MIN/MAX aggregates.  Stop as soon as we find one that isn't.
+	 * Examine all the aggregates and verify all are MIN/MAX aggregates.  Stop
+	 * as soon as we find one that isn't.
 	 */
 	aggs_list = NIL;
 	if (!can_minmax_aggs(root, &aggs_list))
@@ -227,24 +227,24 @@ preprocess_minmax_aggregates(PlannerInfo *root)
 
 /*
  * can_minmax_aggs
- *		Walk through all the aggregates in the query, and check
- *		if they are all MIN/MAX aggregates.  If so, build a list of the
- *		distinct aggregate calls in the tree.
+ *		Examine all the aggregates in the query, and check if they are
+ *		all MIN/MAX aggregates.  If so, build a list of MinMaxAggInfo
+ *		nodes for them.
  *
  * Returns false if a non-MIN/MAX aggregate is found, true otherwise.
- *
- * This does not descend into subqueries, and so should be used only after
- * reduction of sublinks to subplans.  There mustn't be outer-aggregate
- * references either.
  */
 static bool
 can_minmax_aggs(PlannerInfo *root, List **context)
 {
 	ListCell   *lc;
 
+	/*
+	 * This function used to have to scan the query for itself, but now we can
+	 * just thumb through the AggInfo list made by preprocess_aggrefs.
+	 */
 	foreach(lc, root->agginfos)
 	{
-		AggInfo    *agginfo = (AggInfo *) lfirst(lc);
+		AggInfo    *agginfo = lfirst_node(AggInfo, lc);
 		Aggref	   *aggref = agginfo->representative_aggref;
 		Oid			aggsortop;
 		TargetEntry *curTarget;
diff --git a/src/backend/optimizer/prep/prepagg.c b/src/backend/optimizer/prep/prepagg.c
index 404a5f1dac8..5b12937eada 100644
--- a/src/backend/optimizer/prep/prepagg.c
+++ b/src/backend/optimizer/prep/prepagg.c
@@ -223,13 +223,13 @@ preprocess_aggref(Aggref *aggref, PlannerInfo *root)
 	aggno = find_compatible_agg(root, aggref, &same_input_transnos);
 	if (aggno != -1)
 	{
-		AggInfo    *agginfo = list_nth(root->agginfos, aggno);
+		AggInfo    *agginfo = list_nth_node(AggInfo, root->agginfos, aggno);
 
 		transno = agginfo->transno;
 	}
 	else
 	{
-		AggInfo    *agginfo = palloc(sizeof(AggInfo));
+		AggInfo    *agginfo = makeNode(AggInfo);
 
 		agginfo->finalfn_oid = aggfinalfn;
 		agginfo->representative_aggref = aggref;
@@ -266,7 +266,7 @@ preprocess_aggref(Aggref *aggref, PlannerInfo *root)
 										same_input_transnos);
 		if (transno == -1)
 		{
-			AggTransInfo *transinfo = palloc(sizeof(AggTransInfo));
+			AggTransInfo *transinfo = makeNode(AggTransInfo);
 
 			transinfo->args = aggref->args;
 			transinfo->aggfilter = aggref->aggfilter;
@@ -381,7 +381,7 @@ find_compatible_agg(PlannerInfo *root, Aggref *newagg,
 	aggno = -1;
 	foreach(lc, root->agginfos)
 	{
-		AggInfo    *agginfo = (AggInfo *) lfirst(lc);
+		AggInfo    *agginfo = lfirst_node(AggInfo, lc);
 		Aggref	   *existingRef;
 
 		aggno++;
@@ -452,7 +452,9 @@ find_compatible_trans(PlannerInfo *root, Aggref *newagg, bool shareable,
 	foreach(lc, transnos)
 	{
 		int			transno = lfirst_int(lc);
-		AggTransInfo *pertrans = (AggTransInfo *) list_nth(root->aggtransinfos, transno);
+		AggTransInfo *pertrans = list_nth_node(AggTransInfo,
+											   root->aggtransinfos,
+											   transno);
 
 		/*
 		 * if the transfns or transition state types are not the same then the
@@ -541,7 +543,7 @@ get_agg_clause_costs(PlannerInfo *root, AggSplit aggsplit, AggClauseCosts *costs
 
 	foreach(lc, root->aggtransinfos)
 	{
-		AggTransInfo *transinfo = (AggTransInfo *) lfirst(lc);
+		AggTransInfo *transinfo = lfirst_node(AggTransInfo, lc);
 
 		/*
 		 * Add the appropriate component function execution costs to
@@ -645,7 +647,7 @@ get_agg_clause_costs(PlannerInfo *root, AggSplit aggsplit, AggClauseCosts *costs
 
 	foreach(lc, root->agginfos)
 	{
-		AggInfo    *agginfo = (AggInfo *) lfirst(lc);
+		AggInfo    *agginfo = lfirst_node(AggInfo, lc);
 		Aggref	   *aggref = agginfo->representative_aggref;
 
 		/*
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 69ba254372d..e650af5ff29 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -442,15 +442,15 @@ struct PlannerInfo
 	 * Information about aggregates. Filled by preprocess_aggrefs().
 	 */
 	/* AggInfo structs */
-	List	   *agginfos pg_node_attr(read_write_ignore);
+	List	   *agginfos;
 	/* AggTransInfo structs */
-	List	   *aggtransinfos pg_node_attr(read_write_ignore);
-	/* number w/ DISTINCT/ORDER BY/WITHIN GROUP */
-	int			numOrderedAggs pg_node_attr(read_write_ignore);
+	List	   *aggtransinfos;
+	/* number of aggs with DISTINCT/ORDER BY/WITHIN GROUP */
+	int			numOrderedAggs;
 	/* does any agg not support partial mode? */
-	bool		hasNonPartialAggs pg_node_attr(read_write_ignore);
+	bool		hasNonPartialAggs;
 	/* is any partial agg non-serializable? */
-	bool		hasNonSerialAggs pg_node_attr(read_write_ignore);
+	bool		hasNonSerialAggs;
 
 	/*
 	 * These fields are used only when hasRecursion is true:
@@ -3121,6 +3121,10 @@ typedef struct JoinCostWorkspace
  */
 typedef struct AggInfo
 {
+	pg_node_attr(no_copy_equal, no_read)
+
+	NodeTag		type;
+
 	/*
 	 * Link to an Aggref expr this state value is for.
 	 *
@@ -3129,6 +3133,7 @@ typedef struct AggInfo
 	 */
 	Aggref	   *representative_aggref;
 
+	/* Transition state number for this aggregate */
 	int			transno;
 
 	/*
@@ -3137,9 +3142,8 @@ typedef struct AggInfo
 	 */
 	bool		shareable;
 
-	/* Oid of the final function or InvalidOid */
+	/* Oid of the final function, or InvalidOid if none */
 	Oid			finalfn_oid;
-
 } AggInfo;
 
 /*
@@ -3151,34 +3155,40 @@ typedef struct AggInfo
  */
 typedef struct AggTransInfo
 {
+	pg_node_attr(no_copy_equal, no_read)
+
+	NodeTag		type;
+
+	/* Inputs for this transition state */
 	List	   *args;
 	Expr	   *aggfilter;
 
 	/* Oid of the state transition function */
 	Oid			transfn_oid;
 
-	/* Oid of the serialization function or InvalidOid */
+	/* Oid of the serialization function, or InvalidOid if none */
 	Oid			serialfn_oid;
 
-	/* Oid of the deserialization function or InvalidOid */
+	/* Oid of the deserialization function, or InvalidOid if none */
 	Oid			deserialfn_oid;
 
-	/* Oid of the combine function or InvalidOid */
+	/* Oid of the combine function, or InvalidOid if none */
 	Oid			combinefn_oid;
 
 	/* Oid of state value's datatype */
 	Oid			aggtranstype;
+
+	/* Additional data about transtype */
 	int32		aggtranstypmod;
 	int			transtypeLen;
 	bool		transtypeByVal;
+
+	/* Space-consumption estimate */
 	int32		aggtransspace;
 
-	/*
-	 * initial value from pg_aggregate entry
-	 */
-	Datum		initValue;
+	/* Initial value from pg_aggregate entry */
+	Datum		initValue pg_node_attr(read_write_ignore);
 	bool		initValueIsNull;
-
 } AggTransInfo;
 
 #endif							/* PATHNODES_H */