diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c index dfb51f609d..169bf6fcfe 100644 --- a/src/backend/access/gist/gist.c +++ b/src/backend/access/gist/gist.c @@ -1323,8 +1323,6 @@ static void gistfinishsplit(GISTInsertState *state, GISTInsertStack *stack, GISTSTATE *giststate, List *splitinfo, bool unlockbuf) { - ListCell *lc; - List *reversed; GISTPageSplitInfo *right; GISTPageSplitInfo *left; IndexTuple tuples[2]; @@ -1339,14 +1337,6 @@ gistfinishsplit(GISTInsertState *state, GISTInsertStack *stack, * left. Finally insert the downlink for the last new page and update the * downlink for the original page as one operation. */ - - /* for convenience, create a copy of the list in reverse order */ - reversed = NIL; - foreach(lc, splitinfo) - { - reversed = lcons(lfirst(lc), reversed); - } - LockBuffer(stack->parent->buffer, GIST_EXCLUSIVE); gistFindCorrectParent(state->r, stack); @@ -1354,10 +1344,10 @@ gistfinishsplit(GISTInsertState *state, GISTInsertStack *stack, * insert downlinks for the siblings from right to left, until there are * only two siblings left. */ - while (list_length(reversed) > 2) + for (int pos = list_length(splitinfo) - 1; pos > 1; pos--) { - right = (GISTPageSplitInfo *) linitial(reversed); - left = (GISTPageSplitInfo *) lsecond(reversed); + right = (GISTPageSplitInfo *) list_nth(splitinfo, pos); + left = (GISTPageSplitInfo *) list_nth(splitinfo, pos - 1); if (gistinserttuples(state, stack->parent, giststate, &right->downlink, 1, @@ -1371,11 +1361,10 @@ gistfinishsplit(GISTInsertState *state, GISTInsertStack *stack, gistFindCorrectParent(state->r, stack); } /* gistinserttuples() released the lock on right->buf. */ - reversed = list_delete_first(reversed); } - right = (GISTPageSplitInfo *) linitial(reversed); - left = (GISTPageSplitInfo *) lsecond(reversed); + right = (GISTPageSplitInfo *) lsecond(splitinfo); + left = (GISTPageSplitInfo *) linitial(splitinfo); /* * Finally insert downlink for the remaining right page and update the diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 6c3ff762df..032fab9ac4 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -633,7 +633,7 @@ CheckAttributeType(const char *attname, errmsg("composite type %s cannot be made a member of itself", format_type_be(atttypid)))); - containing_rowtypes = lcons_oid(atttypid, containing_rowtypes); + containing_rowtypes = lappend_oid(containing_rowtypes, atttypid); relation = relation_open(get_typ_typrelid(atttypid), AccessShareLock); @@ -653,7 +653,7 @@ CheckAttributeType(const char *attname, relation_close(relation, AccessShareLock); - containing_rowtypes = list_delete_first(containing_rowtypes); + containing_rowtypes = list_delete_last(containing_rowtypes); } else if (OidIsValid((att_typelem = get_element_type(atttypid)))) { diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index ebaec4f8dd..cedb4ee844 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -1566,7 +1566,7 @@ get_tables_to_cluster(MemoryContext cluster_context) rvtc = (RelToCluster *) palloc(sizeof(RelToCluster)); rvtc->tableOid = index->indrelid; rvtc->indexOid = index->indexrelid; - rvs = lcons(rvtc, rvs); + rvs = lappend(rvs, rvtc); MemoryContextSwitchTo(old_context); } diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.c index 417d595a7f..bae3b38f77 100644 --- a/src/backend/commands/lockcmds.c +++ b/src/backend/commands/lockcmds.c @@ -281,11 +281,11 @@ LockViewRecurse(Oid reloid, LOCKMODE lockmode, bool nowait, List *ancestor_views context.nowait = nowait; context.viewowner = view->rd_rel->relowner; context.viewoid = reloid; - context.ancestor_views = lcons_oid(reloid, ancestor_views); + context.ancestor_views = lappend_oid(ancestor_views, reloid); LockViewRecurse_walker((Node *) viewquery, &context); - ancestor_views = list_delete_oid(ancestor_views, reloid); + (void) list_delete_last(context.ancestor_views); table_close(view, NoLock); } diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 0c0ddd58c5..fc1c4dfa4c 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -14480,6 +14480,11 @@ register_on_commit_action(Oid relid, OnCommitAction action) oc->creating_subid = GetCurrentSubTransactionId(); oc->deleting_subid = InvalidSubTransactionId; + /* + * We use lcons() here so that ON COMMIT actions are processed in reverse + * order of registration. That might not be essential but it seems + * reasonable. + */ on_commits = lcons(oc, on_commits); MemoryContextSwitchTo(oldcxt); diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index e9c8873ade..89887b8fd7 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -2999,7 +2999,7 @@ get_rels_with_domain(Oid domainOid, LOCKMODE lockmode) rtc->rel = rel; rtc->natts = 0; rtc->atts = (int *) palloc(sizeof(int) * RelationGetNumberOfAttributes(rel)); - result = lcons(rtc, result); + result = lappend(result, rtc); } /* diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index e4e05753ee..6d09f2a218 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -786,7 +786,7 @@ ExecInitExprRec(Expr *node, ExprState *state, { AggState *aggstate = (AggState *) state->parent; - aggstate->aggs = lcons(astate, aggstate->aggs); + aggstate->aggs = lappend(aggstate->aggs, astate); aggstate->numaggs++; } else @@ -834,7 +834,7 @@ ExecInitExprRec(Expr *node, ExprState *state, WindowAggState *winstate = (WindowAggState *) state->parent; int nfuncs; - winstate->funcs = lcons(wfstate, winstate->funcs); + winstate->funcs = lappend(winstate->funcs, wfstate); nfuncs = ++winstate->numfuncs; if (wfunc->winagg) winstate->numaggs++; diff --git a/src/backend/nodes/list.c b/src/backend/nodes/list.c index 5584fa87c2..9163464de2 100644 --- a/src/backend/nodes/list.c +++ b/src/backend/nodes/list.c @@ -826,6 +826,30 @@ list_delete_first(List *list) return list_delete_nth_cell(list, 0); } +/* + * Delete the last element of the list. + * + * This is the opposite of list_delete_first(), but is noticeably cheaper + * with a long list, since no data need be moved. + */ +List * +list_delete_last(List *list) +{ + check_list_invariants(list); + + if (list == NIL) + return NIL; /* would an error be better? */ + + /* list_truncate won't free list if it goes to empty, but this should */ + if (list_length(list) <= 1) + { + list_free(list); + return NIL; + } + + return list_truncate(list, list_length(list) - 1); +} + /* * Generate the union of two lists. This is calculated by copying * list1 via list_copy(), then adding to it all the members of list2 diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index f0e789f37e..99dbf8da18 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -881,11 +881,9 @@ is_parallel_safe(PlannerInfo *root, Node *node) foreach(l, proot->init_plans) { SubPlan *initsubplan = (SubPlan *) lfirst(l); - ListCell *l2; - foreach(l2, initsubplan->setParam) - context.safe_param_ids = lcons_int(lfirst_int(l2), - context.safe_param_ids); + context.safe_param_ids = list_concat(context.safe_param_ids, + initsubplan->setParam); } } @@ -1015,6 +1013,7 @@ max_parallel_hazard_walker(Node *node, max_parallel_hazard_context *context) context->safe_param_ids); if (max_parallel_hazard_walker(subplan->testexpr, context)) return true; /* no need to restore safe_param_ids */ + list_free(context->safe_param_ids); context->safe_param_ids = save_safe_param_ids; /* we must also check args, but no special Param treatment there */ if (max_parallel_hazard_walker((Node *) subplan->args, context)) @@ -4185,8 +4184,8 @@ add_function_defaults(List *args, HeapTuple func_tuple) ndelete = nargsprovided + list_length(defaults) - funcform->pronargs; if (ndelete < 0) elog(ERROR, "not enough default arguments"); - while (ndelete-- > 0) - defaults = list_delete_first(defaults); + if (ndelete > 0) + defaults = list_copy_tail(defaults, ndelete); /* And form the combined argument list, not modifying the input list */ return list_concat(list_copy(args), defaults); @@ -4701,9 +4700,9 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid, * Recursively try to simplify the modified expression. Here we must add * the current function to the context list of active functions. */ - context->active_fns = lcons_oid(funcid, context->active_fns); + context->active_fns = lappend_oid(context->active_fns, funcid); newexpr = eval_const_expressions_mutator(newexpr, context); - context->active_fns = list_delete_first(context->active_fns); + context->active_fns = list_delete_last(context->active_fns); error_context_stack = sqlerrcontext.previous; diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c index 6ea625a148..98e99481c6 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -419,6 +419,13 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent, index_close(indexRelation, NoLock); + /* + * We've historically used lcons() here. It'd make more sense to + * use lappend(), but that causes the planner to change behavior + * in cases where two indexes seem equally attractive. For now, + * stick with lcons() --- few tables should have so many indexes + * that the O(N^2) behavior of lcons() is really a problem. + */ indexinfos = lcons(info, indexinfos); } @@ -1339,7 +1346,7 @@ get_relation_statistics(RelOptInfo *rel, Relation relation) info->kind = STATS_EXT_NDISTINCT; info->keys = bms_copy(keys); - stainfos = lcons(info, stainfos); + stainfos = lappend(stainfos, info); } if (statext_is_kind_built(dtup, STATS_EXT_DEPENDENCIES)) @@ -1351,7 +1358,7 @@ get_relation_statistics(RelOptInfo *rel, Relation relation) info->kind = STATS_EXT_DEPENDENCIES; info->keys = bms_copy(keys); - stainfos = lcons(info, stainfos); + stainfos = lappend(stainfos, info); } if (statext_is_kind_built(dtup, STATS_EXT_MCV)) @@ -1363,7 +1370,7 @@ get_relation_statistics(RelOptInfo *rel, Relation relation) info->kind = STATS_EXT_MCV; info->keys = bms_copy(keys); - stainfos = lcons(info, stainfos); + stainfos = lappend(stainfos, info); } ReleaseSysCache(htup); diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c index 19e3164afb..354030e549 100644 --- a/src/backend/parser/parse_agg.c +++ b/src/backend/parser/parse_agg.c @@ -1132,7 +1132,7 @@ parseCheckAggregates(ParseState *pstate, Query *qry) if (expr == NULL) continue; /* probably cannot happen */ - groupClauses = lcons(expr, groupClauses); + groupClauses = lappend(groupClauses, expr); } /* diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 5b047d1662..93b6784023 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -1973,7 +1973,7 @@ fireRIRrules(Query *parsetree, List *activeRIRs) (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), errmsg("infinite recursion detected in rules for relation \"%s\"", RelationGetRelationName(rel)))); - activeRIRs = lcons_oid(RelationGetRelid(rel), activeRIRs); + activeRIRs = lappend_oid(activeRIRs, RelationGetRelid(rel)); foreach(l, locks) { @@ -1986,7 +1986,7 @@ fireRIRrules(Query *parsetree, List *activeRIRs) activeRIRs); } - activeRIRs = list_delete_first(activeRIRs); + activeRIRs = list_delete_last(activeRIRs); } } @@ -2059,7 +2059,7 @@ fireRIRrules(Query *parsetree, List *activeRIRs) errmsg("infinite recursion detected in policy for relation \"%s\"", RelationGetRelationName(rel)))); - activeRIRs = lcons_oid(RelationGetRelid(rel), activeRIRs); + activeRIRs = lappend_oid(activeRIRs, RelationGetRelid(rel)); /* * get_row_security_policies just passed back securityQuals @@ -2084,7 +2084,7 @@ fireRIRrules(Query *parsetree, List *activeRIRs) expression_tree_walker((Node *) withCheckOptions, fireRIRonSubLink, (void *) activeRIRs); - activeRIRs = list_delete_first(activeRIRs); + activeRIRs = list_delete_last(activeRIRs); } /* @@ -3711,7 +3711,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events) rev = (rewrite_event *) palloc(sizeof(rewrite_event)); rev->relation = RelationGetRelid(rt_entry_relation); rev->event = event; - rewrite_events = lcons(rev, rewrite_events); + rewrite_events = lappend(rewrite_events, rev); foreach(n, product_queries) { @@ -3722,7 +3722,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events) rewritten = list_concat(rewritten, newstuff); } - rewrite_events = list_delete_first(rewrite_events); + rewrite_events = list_delete_last(rewrite_events); } /* diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c index 66449b85b1..7eba59eff3 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -3201,7 +3201,7 @@ estimate_num_groups(PlannerInfo *root, List *groupExprs, double input_rows, * Split the list of varinfos in two - one for the current rel, one * for remaining Vars on other rels. */ - relvarinfos = lcons(varinfo1, relvarinfos); + relvarinfos = lappend(relvarinfos, varinfo1); for_each_cell(l, varinfos, list_second_cell(varinfos)) { GroupVarInfo *varinfo2 = (GroupVarInfo *) lfirst(l); @@ -3209,12 +3209,12 @@ estimate_num_groups(PlannerInfo *root, List *groupExprs, double input_rows, if (varinfo2->rel == varinfo1->rel) { /* varinfos on current rel */ - relvarinfos = lcons(varinfo2, relvarinfos); + relvarinfos = lappend(relvarinfos, varinfo2); } else { /* not time to process varinfo2 yet */ - newvarinfos = lcons(varinfo2, newvarinfos); + newvarinfos = lappend(newvarinfos, varinfo2); } } diff --git a/src/include/nodes/pg_list.h b/src/include/nodes/pg_list.h index 71dc4dc33a..146340877c 100644 --- a/src/include/nodes/pg_list.h +++ b/src/include/nodes/pg_list.h @@ -531,6 +531,7 @@ extern List *list_delete_ptr(List *list, void *datum); extern List *list_delete_int(List *list, int datum); extern List *list_delete_oid(List *list, Oid datum); extern List *list_delete_first(List *list); +extern List *list_delete_last(List *list); extern List *list_delete_nth_cell(List *list, int n); extern List *list_delete_cell(List *list, ListCell *cell); diff --git a/src/test/regress/expected/aggregates.out b/src/test/regress/expected/aggregates.out index ef8eec3fbf..be4ddf86a4 100644 --- a/src/test/regress/expected/aggregates.out +++ b/src/test/regress/expected/aggregates.out @@ -2030,10 +2030,10 @@ NOTICE: avg_transfn called with 3 -- this should not share the state due to different input columns. select my_avg(one),my_sum(two) from (values(1,2),(3,4)) t(one,two); -NOTICE: avg_transfn called with 2 NOTICE: avg_transfn called with 1 -NOTICE: avg_transfn called with 4 +NOTICE: avg_transfn called with 2 NOTICE: avg_transfn called with 3 +NOTICE: avg_transfn called with 4 my_avg | my_sum --------+-------- 2 | 6