From c245776906b065fcd59831a25c3b24ad3ddcd849 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 16 Jul 2019 13:12:24 -0400 Subject: [PATCH] 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 --- src/backend/nodes/list.c | 47 -------------------------- src/backend/optimizer/geqo/geqo_eval.c | 17 ++++------ src/include/nodes/pg_list.h | 4 --- 3 files changed, 6 insertions(+), 62 deletions(-) diff --git a/src/backend/nodes/list.c b/src/backend/nodes/list.c index 0b8686d262..5584fa87c2 100644 --- 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 8a44ac8530..7b67a29c88 100644 --- 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 418f000629..71dc4dc33a 100644 --- 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); -- 2.40.0