From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sun, 13 Mar 2011 16:57:14 +0000 (-0400)
Subject: Simplify list traversal logic in add_path().
X-Git-Tag: REL9_1_ALPHA5~88
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a2eb9e0c08ee73208b5419f5a53a6eba55809b92;p=postgresql

Simplify list traversal logic in add_path().

Its mechanism for recovering after deleting the current list cell was
a bit klugy.  Borrow the technique used in other places.
---

diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 1158f7715b..b60c88d925 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -254,8 +254,9 @@ add_path(RelOptInfo *parent_rel, Path *new_path)
 {
 	bool		accept_new = true;		/* unless we find a superior old path */
 	ListCell   *insert_after = NULL;	/* where to insert new item */
-	ListCell   *p1_prev = NULL;
 	ListCell   *p1;
+	ListCell   *p1_prev;
+	ListCell   *p1_next;
 
 	/*
 	 * This is a convenient place to check for query cancel --- no part of the
@@ -267,14 +268,19 @@ add_path(RelOptInfo *parent_rel, Path *new_path)
 	 * Loop to check proposed new path against old paths.  Note it is possible
 	 * for more than one old path to be tossed out because new_path dominates
 	 * it.
+	 *
+	 * We can't use foreach here because the loop body may delete the current
+	 * list cell.
 	 */
-	p1 = list_head(parent_rel->pathlist);		/* cannot use foreach here */
-	while (p1 != NULL)
+	p1_prev = NULL;
+	for (p1 = list_head(parent_rel->pathlist); p1 != NULL; p1 = p1_next)
 	{
 		Path	   *old_path = (Path *) lfirst(p1);
 		bool		remove_old = false; /* unless new proves superior */
 		int			costcmp;
 
+		p1_next = lnext(p1);
+
 		/*
 		 * As of Postgres 8.0, we use fuzzy cost comparison to avoid wasting
 		 * cycles keeping paths that are really not significantly different in
@@ -343,20 +349,15 @@ add_path(RelOptInfo *parent_rel, Path *new_path)
 			 */
 			if (!IsA(old_path, IndexPath))
 				pfree(old_path);
-			/* Advance list pointer */
-			if (p1_prev)
-				p1 = lnext(p1_prev);
-			else
-				p1 = list_head(parent_rel->pathlist);
+			/* p1_prev does not advance */
 		}
 		else
 		{
 			/* new belongs after this old path if it has cost >= old's */
 			if (costcmp >= 0)
 				insert_after = p1;
-			/* Advance list pointers */
+			/* p1_prev advances */
 			p1_prev = p1;
-			p1 = lnext(p1);
 		}
 
 		/*