Simplify list traversal logic in add_path().
authorTom Lane <[email protected]>
Sun, 13 Mar 2011 16:57:14 +0000 (12:57 -0400)
committerTom Lane <[email protected]>
Sun, 13 Mar 2011 16:57:14 +0000 (12:57 -0400)
Its mechanism for recovering after deleting the current list cell was
a bit klugy.  Borrow the technique used in other places.

src/backend/optimizer/util/pathnode.c

index 1158f7715bcd29cd0039d1298c29206973148f79..b60c88d9251222a9e0a0944dd3ac23f6b3f7c061 100644 (file)
@@ -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);
        }
 
        /*