refactoring, extract 'Relation parent_rel' from append_child_relation(), use list_free_deep() instead of list_free()

funbringer · funbringer · commit 2d224b651ebe · 2016-10-21T16:10:03.000+03:00
diff --git a/src/hooks.c b/src/hooks.c
@@ -209,22 +209,22 @@ pathman_rel_pathlist_hook(PlannerInfo *root,
 	/* Proceed iff relation 'rel' is partitioned */
 	if ((prel = get_pathman_relation_info(rte->relid)) != NULL)
 	{
-		ListCell	   *lc;
-		Oid			   *children;
-		List		   *ranges,
-					   *wrappers,
-					   *rel_part_clauses = NIL;
+		Relation		parent_rel;				/* parent's relation (heap */
+		Oid			   *children;				/* selected children oids */
+		List		   *ranges,					/* a list of IndexRanges */
+					   *wrappers,				/* a list of WrapperNodes */
+					   *rel_part_clauses = NIL;	/* clauses with part. column */
 		PathKey		   *pathkeyAsc = NULL,
 					   *pathkeyDesc = NULL;
-		double			paramsel = 1.0;
+		double			paramsel = 1.0;			/* default part selectivity */
 		WalkerContext	context;
+		ListCell	   *lc;
 		int				i;
 
 		if (prel->parttype == PT_RANGE)
 		{
 			/*
-			 * Get pathkeys for ascending and descending sort by partition
-			 * column
+			 * Get pathkeys for ascending and descending sort by partitioned column.
 			 */
 			List		   *pathkeys;
 			Var			   *var;
@@ -273,13 +273,12 @@ pathman_rel_pathlist_hook(PlannerInfo *root,
 			ranges = irange_list_intersect(ranges, wrap->rangeset);
 		}
 
-		/*
-		 * Expand simple_rte_array and simple_rel_array
-		 */
+		/* Get number of selected partitions */
 		len = irange_list_length(ranges);
 		if (prel->enable_parent)
-			len++;
+			len++; /* add parent too */
 
+		/* Expand simple_rte_array and simple_rel_array */
 		if (len > 0)
 		{
 			/* Expand simple_rel_array and simple_rte_array */
@@ -306,26 +305,32 @@ pathman_rel_pathlist_hook(PlannerInfo *root,
 			root->simple_rte_array = new_rte_array;
 		}
 
-		/* Add parent if needed */
+		/* Parent has already been locked by rewriter */
+		parent_rel = heap_open(rte->relid, NoLock);
+
+		/* Add parent if asked to */
 		if (prel->enable_parent)
-			append_child_relation(root, rti, 0, rte->relid, NULL);
+			append_child_relation(root, parent_rel, rti, 0, rte->relid, NULL);
 
 		/*
-		 * Iterate all indexes in rangeset and append corresponding child
-		 * relations.
+		 * Iterate all indexes in rangeset and append corresponding child relations.
 		 */
 		foreach(lc, ranges)
 		{
 			IndexRange irange = lfirst_irange(lc);
 
 			for (i = irange.ir_lower; i <= irange.ir_upper; i++)
-				append_child_relation(root, rti, i, children[i], wrappers);
+				append_child_relation(root, parent_rel, rti, i, children[i], wrappers);
 		}
 
-		/* Clear old path list */
-		list_free(rel->pathlist);
+		/* Now close parent relation */
+		heap_close(parent_rel, NoLock);
 
+		/* Clear path list and make it point to NIL */
+		list_free_deep(rel->pathlist);
 		rel->pathlist = NIL;
+
+		/* Generate new paths using the rels we've just added */
 		set_append_rel_pathlist(root, rel, rti, pathkeyAsc, pathkeyDesc);
 		set_append_rel_size_compat(root, rel, rti);
 
@@ -342,6 +347,7 @@ pathman_rel_pathlist_hook(PlannerInfo *root,
 		if (!clause_contains_params((Node *) rel_part_clauses))
 			return;
 
+		/* Generate Runtime[Merge]Append paths if needed */
 		foreach (lc, rel->pathlist)
 		{
 			AppendPath	   *cur_path = (AppendPath *) lfirst(lc);
diff --git a/src/pathman.h b/src/pathman.h
@@ -123,8 +123,9 @@ extern List			   *inheritance_disabled_relids;
 extern PathmanState    *pmstate;
 
 
-int append_child_relation(PlannerInfo *root, Index parent_rti,
-						  int ir_index, Oid child_oid, List *wrappers);
+int append_child_relation(PlannerInfo *root, Relation parent_relation,
+						  Index parent_rti, int ir_index, Oid child_oid,
+						  List *wrappers);
 
 search_rangerel_result search_range_partition_eq(const Datum value,
 												 FmgrInfo *cmp_func,
diff --git a/src/pg_pathman.c b/src/pg_pathman.c
@@ -436,15 +436,15 @@ make_inh_translation_list_simplified(Relation oldrelation,
  * NOTE: This code is partially based on the expand_inherited_rtentry() function.
  */
 int
-append_child_relation(PlannerInfo *root, Index parent_rti,
-					  int ir_index, Oid child_oid, List *wrappers)
+append_child_relation(PlannerInfo *root, Relation parent_relation,
+					  Index parent_rti, int ir_index, Oid child_oid,
+					  List *wrappers)
 {
 	RangeTblEntry  *parent_rte,
 				   *child_rte;
 	RelOptInfo	   *parent_rel,
 				   *child_rel;
-	Relation		parent_relation,
-					child_relation;
+	Relation		child_relation;
 	AppendRelInfo  *appinfo;
 	Index			childRTindex;
 	PlanRowMark	   *parent_rowmark,
@@ -455,8 +455,6 @@ append_child_relation(PlannerInfo *root, Index parent_rti,
 	parent_rel = root->simple_rel_array[parent_rti];
 	parent_rte = root->simple_rte_array[parent_rti];
 
-	/* Parent has already been locked by rewriter */
-	parent_relation = heap_open(parent_rte->relid, NoLock);
 	/* FIXME: acquire a suitable lock on partition */
 	child_relation = heap_open(child_oid, NoLock);
 
@@ -552,7 +550,6 @@ append_child_relation(PlannerInfo *root, Index parent_rti,
 	parent_rel->tuples += child_rel->tuples;
 
 	/* Close child relations, but keep locks */
-	heap_close(parent_relation, NoLock);
 	heap_close(child_relation, NoLock);
 
 
@@ -600,6 +597,7 @@ wrapper_make_expression(WrapperNode *wrap, int index, bool *alwaysTrue)
 	/* Return NULL for always true and always false. */
 	if (!found)
 		return NULL;
+
 	if (!lossy)
 	{
 		*alwaysTrue = true;
