From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 13 Jun 2016 16:59:25 +0000 (-0400)
Subject: In planner.c, avoid assuming that all PathTargets have sortgrouprefs.
X-Git-Tag: REL9_6_BETA2~48
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=89d53515e53ea080029894939118365b647489b3;p=postgresql

In planner.c, avoid assuming that all PathTargets have sortgrouprefs.

The struct definition for PathTarget specifies that a NULL sortgrouprefs
pointer means no sortgroupref labels.  While it's likely that there
should always be at least one labeled column in the places that were
unconditionally fetching through the pointer, it seems wiser to adhere to
the data structure specification and test first.  Add a macro to make this
convenient.  Per experimentation with running the regression tests with a
very small parallelization threshold --- the crash I observed may well
represent a bug elsewhere, but still this coding was not very robust.

Report: <20756.1465834072@sss.pgh.pa.us>
---

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 54c0440436..07b925e74c 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -4222,7 +4222,7 @@ make_group_input_target(PlannerInfo *root, PathTarget *final_target)
 	foreach(lc, final_target->exprs)
 	{
 		Expr	   *expr = (Expr *) lfirst(lc);
-		Index		sgref = final_target->sortgrouprefs[i];
+		Index		sgref = get_pathtarget_sortgroupref(final_target, i);
 
 		if (sgref && parse->groupClause &&
 			get_sortgroupref_clause_noerr(sgref, parse->groupClause) != NULL)
@@ -4304,7 +4304,7 @@ make_partialgroup_input_target(PlannerInfo *root, PathTarget *final_target)
 	foreach(lc, final_target->exprs)
 	{
 		Expr	   *expr = (Expr *) lfirst(lc);
-		Index		sgref = final_target->sortgrouprefs[i];
+		Index		sgref = get_pathtarget_sortgroupref(final_target, i);
 
 		if (sgref && parse->groupClause &&
 			get_sortgroupref_clause_noerr(sgref, parse->groupClause) != NULL)
@@ -4556,7 +4556,7 @@ make_window_input_target(PlannerInfo *root,
 	foreach(lc, final_target->exprs)
 	{
 		Expr	   *expr = (Expr *) lfirst(lc);
-		Index		sgref = final_target->sortgrouprefs[i];
+		Index		sgref = get_pathtarget_sortgroupref(final_target, i);
 
 		/*
 		 * Don't want to deconstruct window clauses or GROUP BY items.  (Note
@@ -4757,7 +4757,7 @@ make_sort_input_target(PlannerInfo *root,
 		 * only be Vars anyway.  There don't seem to be any cases where it
 		 * would be worth the trouble to double-check.
 		 */
-		if (final_target->sortgrouprefs[i] == 0)
+		if (get_pathtarget_sortgroupref(final_target, i) == 0)
 		{
 			/*
 			 * Check for SRF or volatile functions.  Check the SRF case first
@@ -4847,7 +4847,7 @@ make_sort_input_target(PlannerInfo *root,
 			postponable_cols = lappend(postponable_cols, expr);
 		else
 			add_column_to_pathtarget(input_target, expr,
-									 final_target->sortgrouprefs[i]);
+							   get_pathtarget_sortgroupref(final_target, i));
 
 		i++;
 	}
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index cafd696229..a4892cbae5 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -783,6 +783,10 @@ typedef struct PathTarget
 	int			width;			/* estimated avg width of result tuples */
 } PathTarget;
 
+/* Convenience macro to get a sort/group refno from a PathTarget */
+#define get_pathtarget_sortgroupref(target, colno) \
+	((target)->sortgrouprefs ? (target)->sortgrouprefs[colno] : (Index) 0)
+
 
 /*
  * ParamPathInfo