Remove lappend_cell...() family of List functions.

It seems worth getting rid of these functions because they require the
caller to retain a ListCell pointer into a List that it's modifying,
which is a dangerous practice with the new List implementation.
(The only other List-modifying function that takes a ListCell pointer
as input is list_delete_cell, which nowadays is preferentially used
via the constrained API foreach_delete_current.)

There was only one remaining caller of these functions after commit
2f5b8eb5a, and that was some fairly ugly GEQO code that can be much
more clearly expressed using a list-index variable and list_insert_nth.
Hence, rewrite that code, and remove the functions.

Discussion: https://postgr.es/m/26193.1563228600@sss.pgh.pa.us

diff --git a/src/backend/nodes/list.c b/src/backend/nodes/list.c
index 0b8686d262d1be8147e01ae3b322e5c3b2e12074..5584fa87c2ddb2fcc3f9a3b17730d8222baf663b 100644 (file)
--- a/src/backend/nodes/list.c
+++ b/src/backend/nodes/list.c
@@ -438,53 +438,6 @@ list_insert_nth_oid(List *list, int pos, Oid datum)
        return list;
 }
 
-/*
- * Add a new cell to the list, in the position after 'prev_cell'. The
- * data in the cell is left undefined, and must be filled in by the
- * caller. 'list' is assumed to be non-NIL, and 'prev_cell' is assumed
- * to be non-NULL and a member of 'list'. Returns address of new cell.
- *
- * Caution: prev_cell might no longer point into the list after this!
- */
-static ListCell *
-add_new_cell_after(List *list, ListCell *prev_cell)
-{
-       /* insert_new_cell will assert that this is in-range: */
-       int                     pos = prev_cell - list->elements;
-
-       return insert_new_cell(list, pos + 1);
-}
-
-/*
- * Add a new cell to the specified list (which must be non-NIL);
- * it will be placed after the list cell 'prev' (which must be
- * non-NULL and a member of 'list'). The data placed in the new cell
- * is 'datum'.
- */
-void
-lappend_cell(List *list, ListCell *prev, void *datum)
-{
-       Assert(IsPointerList(list));
-       lfirst(add_new_cell_after(list, prev)) = datum;
-       check_list_invariants(list);
-}
-
-void
-lappend_cell_int(List *list, ListCell *prev, int datum)
-{
-       Assert(IsIntegerList(list));
-       lfirst_int(add_new_cell_after(list, prev)) = datum;
-       check_list_invariants(list);
-}
-
-void
-lappend_cell_oid(List *list, ListCell *prev, Oid datum)
-{
-       Assert(IsOidList(list));
-       lfirst_oid(add_new_cell_after(list, prev)) = datum;
-       check_list_invariants(list);
-}
-
 /*
  * Prepend a new element to the list. A pointer to the modified list
  * is returned. Note that this function may or may not destructively

diff --git a/src/backend/optimizer/geqo/geqo_eval.c b/src/backend/optimizer/geqo/geqo_eval.c
index 8a44ac8530d7573a5fbba1a1ec122d0e2f493c0d..7b67a29c88aeb906ba943d3089aee59ab49e27c4 100644 (file)
--- a/src/backend/optimizer/geqo/geqo_eval.c
+++ b/src/backend/optimizer/geqo/geqo_eval.c
@@ -239,6 +239,7 @@ merge_clump(PlannerInfo *root, List *clumps, Clump *new_clump, int num_gene,
                        bool force)
 {
        ListCell   *lc;
+       int                     pos;
 
        /* Look for a clump that new_clump can join to */
        foreach(lc, clumps)
@@ -304,21 +305,15 @@ merge_clump(PlannerInfo *root, List *clumps, Clump *new_clump, int num_gene,
        if (clumps == NIL || new_clump->size == 1)
                return lappend(clumps, new_clump);
 
-       /* Check if it belongs at the front */
-       lc = list_head(clumps);
-       if (new_clump->size > ((Clump *) lfirst(lc))->size)
-               return lcons(new_clump, clumps);
-
        /* Else search for the place to insert it */
-       for (;;)
+       for (pos = 0; pos < list_length(clumps); pos++)
        {
-               ListCell   *nxt = lnext(clumps, lc);
+               Clump      *old_clump = (Clump *) list_nth(clumps, pos);
 
-               if (nxt == NULL || new_clump->size > ((Clump *) lfirst(nxt))->size)
-                       break;                          /* it belongs after 'lc', before 'nxt' */
-               lc = nxt;
+               if (new_clump->size > old_clump->size)
+                       break;                          /* new_clump belongs before old_clump */
        }
-       lappend_cell(clumps, lc, new_clump);
+       clumps = list_insert_nth(clumps, pos, new_clump);
 
        return clumps;
 }

diff --git a/src/include/nodes/pg_list.h b/src/include/nodes/pg_list.h
index 418f000629fe810f1470d133e3440883aa6659cb..71dc4dc33a55da0d75890d8a1220ad4ba48399d4 100644 (file)
--- a/src/include/nodes/pg_list.h
+++ b/src/include/nodes/pg_list.h
@@ -514,10 +514,6 @@ extern List *list_insert_nth(List *list, int pos, void *datum);
 extern List *list_insert_nth_int(List *list, int pos, int datum);
 extern List *list_insert_nth_oid(List *list, int pos, Oid datum);
 
-extern void lappend_cell(List *list, ListCell *prev, void *datum);
-extern void lappend_cell_int(List *list, ListCell *prev, int datum);
-extern void lappend_cell_oid(List *list, ListCell *prev, Oid datum);
-
 extern List *lcons(void *datum, List *list);
 extern List *lcons_int(int datum, List *list);
 extern List *lcons_oid(Oid datum, List *list);