Re: POC: converting Lists into arrays - Mailing list pgsql-hackers
| From | Tom Lane |
|---|---|
| Subject | Re: POC: converting Lists into arrays |
| Date | |
| Msg-id | 6704.1563739305@sss.pgh.pa.us Whole thread Raw |
| In response to | Re: POC: converting Lists into arrays (Tom Lane <tgl@sss.pgh.pa.us>) |
| Responses |
Re: POC: converting Lists into arrays
|
| List | pgsql-hackers |
I wrote:
>> * Rationalize places that are using combinations of list_copy and
>> list_concat, probably by inventing an additional list-concatenation
>> primitive that modifies neither input.
> I poked around to see what we have in this department. There seem to
> be several identifiable use-cases:
> [ ... analysis ... ]
Here's a proposed patch based on that. I added list_concat_copy()
and then simplified callers as appropriate.
It turns out there are a *lot* of places where list_concat() callers
are now leaking the second input list (where before they just leaked
that list's header). So I've got mixed emotions about the choice not
to add a variant function that list_free's the second input. On the
other hand, the leakage probably amounts to nothing significant in
all or nearly all of these places, and I'm concerned about the
readability/understandability loss of having an extra version of
list_concat. Anybody have an opinion about that?
Other than that point, I think this is pretty much good to go.
regards, tom lane
diff --git a/contrib/postgres_fdw/deparse.c b/contrib/postgres_fdw/deparse.c
index 548ae66..50d1f18 100644
--- a/contrib/postgres_fdw/deparse.c
+++ b/contrib/postgres_fdw/deparse.c
@@ -1497,7 +1497,7 @@ deparseFromExprForRel(StringInfo buf, PlannerInfo *root, RelOptInfo *foreignrel,
if (fpinfo->jointype == JOIN_INNER)
{
*ignore_conds = list_concat(*ignore_conds,
- list_copy(fpinfo->joinclauses));
+ fpinfo->joinclauses);
fpinfo->joinclauses = NIL;
}
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 033aeb2..b9f90e9 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -2667,8 +2667,8 @@ estimate_path_cost_size(PlannerInfo *root,
* baserestrictinfo plus any extra join_conds relevant to this
* particular path.
*/
- remote_conds = list_concat(list_copy(remote_param_join_conds),
- fpinfo->remote_conds);
+ remote_conds = list_concat_copy(remote_param_join_conds,
+ fpinfo->remote_conds);
/*
* Construct EXPLAIN query including the desired SELECT, FROM, and
@@ -5102,23 +5102,23 @@ foreign_join_ok(PlannerInfo *root, RelOptInfo *joinrel, JoinType jointype,
{
case JOIN_INNER:
fpinfo->remote_conds = list_concat(fpinfo->remote_conds,
- list_copy(fpinfo_i->remote_conds));
+ fpinfo_i->remote_conds);
fpinfo->remote_conds = list_concat(fpinfo->remote_conds,
- list_copy(fpinfo_o->remote_conds));
+ fpinfo_o->remote_conds);
break;
case JOIN_LEFT:
fpinfo->joinclauses = list_concat(fpinfo->joinclauses,
- list_copy(fpinfo_i->remote_conds));
+ fpinfo_i->remote_conds);
fpinfo->remote_conds = list_concat(fpinfo->remote_conds,
- list_copy(fpinfo_o->remote_conds));
+ fpinfo_o->remote_conds);
break;
case JOIN_RIGHT:
fpinfo->joinclauses = list_concat(fpinfo->joinclauses,
- list_copy(fpinfo_o->remote_conds));
+ fpinfo_o->remote_conds);
fpinfo->remote_conds = list_concat(fpinfo->remote_conds,
- list_copy(fpinfo_i->remote_conds));
+ fpinfo_i->remote_conds);
break;
case JOIN_FULL:
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index fd29927..e0af665 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -526,8 +526,8 @@ DefineIndex(Oid relationId,
* is part of the key columns, and anything equal to and over is part of
* the INCLUDE columns.
*/
- allIndexParams = list_concat(list_copy(stmt->indexParams),
- list_copy(stmt->indexIncludingParams));
+ allIndexParams = list_concat_copy(stmt->indexParams,
+ stmt->indexIncludingParams);
numberOfAttributes = list_length(allIndexParams);
if (numberOfAttributes <= 0)
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
index 64a9e58..83337c2 100644
--- a/src/backend/executor/functions.c
+++ b/src/backend/executor/functions.c
@@ -719,8 +719,7 @@ init_sql_fcache(FmgrInfo *finfo, Oid collation, bool lazyEvalOK)
fcache->pinfo,
NULL);
queryTree_list = lappend(queryTree_list, queryTree_sublist);
- flat_query_list = list_concat(flat_query_list,
- list_copy(queryTree_sublist));
+ flat_query_list = list_concat(flat_query_list, queryTree_sublist);
}
check_sql_fn_statements(flat_query_list);
diff --git a/src/backend/nodes/list.c b/src/backend/nodes/list.c
index 9163464..6bf13ae 100644
--- a/src/backend/nodes/list.c
+++ b/src/backend/nodes/list.c
@@ -501,12 +501,15 @@ lcons_oid(Oid datum, List *list)
}
/*
- * Concatenate list2 to the end of list1, and return list1. list1 is
- * destructively changed, list2 is not. (However, in the case of pointer
- * lists, list1 and list2 will point to the same structures.) Callers
- * should be sure to use the return value as the new pointer to the
- * concatenated list: the 'list1' input pointer may or may not be the
- * same as the returned pointer.
+ * Concatenate list2 to the end of list1, and return list1.
+ *
+ * This is equivalent to lappend'ing each element of list2, in order, to list1.
+ * list1 is destructively changed, list2 is not. (However, in the case of
+ * pointer lists, list1 and list2 will point to the same structures.)
+ *
+ * Callers should be sure to use the return value as the new pointer to the
+ * concatenated list: the 'list1' input pointer may or may not be the same
+ * as the returned pointer.
*/
List *
list_concat(List *list1, const List *list2)
@@ -535,6 +538,41 @@ list_concat(List *list1, const List *list2)
}
/*
+ * Form a new list by concatenating the elements of list1 and list2.
+ *
+ * Neither input list is modified. (However, if they are pointer lists,
+ * the output list will point to the same structures.)
+ *
+ * This is equivalent to, but more efficient than,
+ * list_concat(list_copy(list1), list2).
+ * Note that some pre-v13 code might list_copy list2 as well, but that's
+ * pointless now.
+ */
+List *
+list_concat_copy(const List *list1, const List *list2)
+{
+ List *result;
+ int new_len;
+
+ if (list1 == NIL)
+ return list_copy(list2);
+ if (list2 == NIL)
+ return list_copy(list1);
+
+ Assert(list1->type == list2->type);
+
+ new_len = list1->length + list2->length;
+ result = new_list(list1->type, new_len);
+ memcpy(result->elements, list1->elements,
+ list1->length * sizeof(ListCell));
+ memcpy(result->elements + list1->length, list2->elements,
+ list2->length * sizeof(ListCell));
+
+ check_list_invariants(result);
+ return result;
+}
+
+/*
* Truncate 'list' to contain no more than 'new_size' elements. This
* modifies the list in-place! Despite this, callers should use the
* pointer returned by this function to refer to the newly truncated
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index e9ee32b..db3a68a 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -1266,7 +1266,7 @@ set_append_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
if (rel->part_scheme)
rel->partitioned_child_rels =
list_concat(rel->partitioned_child_rels,
- list_copy(childrel->partitioned_child_rels));
+ childrel->partitioned_child_rels);
/*
* Child is live, so add it to the live_childrels list for use below.
@@ -1347,9 +1347,8 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
component = root->simple_rel_array[relid];
Assert(component->part_scheme != NULL);
Assert(list_length(component->partitioned_child_rels) >= 1);
- partrels =
- list_concat(partrels,
- list_copy(component->partitioned_child_rels));
+ partrels = list_concat(partrels,
+ component->partitioned_child_rels);
}
partitioned_rels = list_make1(partrels);
@@ -2048,8 +2047,7 @@ accumulate_append_subpath(Path *path, List **subpaths, List **special_subpaths)
if (!apath->path.parallel_aware || apath->first_partial_path == 0)
{
- /* list_copy is important here to avoid sharing list substructure */
- *subpaths = list_concat(*subpaths, list_copy(apath->subpaths));
+ *subpaths = list_concat(*subpaths, apath->subpaths);
return;
}
else if (special_subpaths != NULL)
@@ -2072,8 +2070,7 @@ accumulate_append_subpath(Path *path, List **subpaths, List **special_subpaths)
{
MergeAppendPath *mpath = (MergeAppendPath *) path;
- /* list_copy is important here to avoid sharing list substructure */
- *subpaths = list_concat(*subpaths, list_copy(mpath->subpaths));
+ *subpaths = list_concat(*subpaths, mpath->subpaths);
return;
}
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 3a9a994..bc6bc99 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -4443,8 +4443,7 @@ get_parameterized_baserel_size(PlannerInfo *root, RelOptInfo *rel,
* restriction clauses. Note that we force the clauses to be treated as
* non-join clauses during selectivity estimation.
*/
- allclauses = list_concat(list_copy(param_clauses),
- rel->baserestrictinfo);
+ allclauses = list_concat_copy(param_clauses, rel->baserestrictinfo);
nrows = rel->tuples *
clauselist_selectivity(root,
allclauses,
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index 5f339fd..37b257c 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -656,7 +656,7 @@ get_join_index_paths(PlannerInfo *root, RelOptInfo *rel,
}
}
- /* Add restriction clauses (this is nondestructive to rclauseset) */
+ /* Add restriction clauses */
clauseset.indexclauses[indexcol] =
list_concat(clauseset.indexclauses[indexcol],
rclauseset->indexclauses[indexcol]);
@@ -1204,8 +1204,7 @@ build_paths_for_OR(PlannerInfo *root, RelOptInfo *rel,
{
/* Form all_clauses if not done already */
if (all_clauses == NIL)
- all_clauses = list_concat(list_copy(clauses),
- other_clauses);
+ all_clauses = list_concat_copy(clauses, other_clauses);
if (!predicate_implied_by(index->indpred, all_clauses, false))
continue; /* can't use it at all */
@@ -1270,7 +1269,7 @@ generate_bitmap_or_paths(PlannerInfo *root, RelOptInfo *rel,
* We can use both the current and other clauses as context for
* build_paths_for_OR; no need to remove ORs from the lists.
*/
- all_clauses = list_concat(list_copy(clauses), other_clauses);
+ all_clauses = list_concat_copy(clauses, other_clauses);
foreach(lc, clauses)
{
@@ -1506,8 +1505,7 @@ choose_bitmap_and(PlannerInfo *root, RelOptInfo *rel, List *paths)
pathinfo = pathinfoarray[i];
paths = list_make1(pathinfo->path);
costsofar = bitmap_scan_cost_est(root, rel, pathinfo->path);
- qualsofar = list_concat(list_copy(pathinfo->quals),
- list_copy(pathinfo->preds));
+ qualsofar = list_concat_copy(pathinfo->quals, pathinfo->preds);
clauseidsofar = bms_copy(pathinfo->clauseids);
for (j = i + 1; j < npaths; j++)
@@ -1543,10 +1541,8 @@ choose_bitmap_and(PlannerInfo *root, RelOptInfo *rel, List *paths)
{
/* keep new path in paths, update subsidiary variables */
costsofar = newcost;
- qualsofar = list_concat(qualsofar,
- list_copy(pathinfo->quals));
- qualsofar = list_concat(qualsofar,
- list_copy(pathinfo->preds));
+ qualsofar = list_concat(qualsofar, pathinfo->quals);
+ qualsofar = list_concat(qualsofar, pathinfo->preds);
clauseidsofar = bms_add_members(clauseidsofar,
pathinfo->clauseids);
}
@@ -1849,7 +1845,7 @@ find_indexpath_quals(Path *bitmapqual, List **quals, List **preds)
*quals = lappend(*quals, iclause->rinfo->clause);
}
- *preds = list_concat(*preds, list_copy(ipath->indexinfo->indpred));
+ *preds = list_concat(*preds, ipath->indexinfo->indpred);
}
else
elog(ERROR, "unrecognized node type: %d", nodeTag(bitmapqual));
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index c6b8553..355b03f 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -556,8 +556,8 @@ create_scan_plan(PlannerInfo *root, Path *best_path, int flags)
* For paranoia's sake, don't modify the stored baserestrictinfo list.
*/
if (best_path->param_info)
- scan_clauses = list_concat(list_copy(scan_clauses),
- best_path->param_info->ppi_clauses);
+ scan_clauses = list_concat_copy(scan_clauses,
+ best_path->param_info->ppi_clauses);
/*
* Detect whether we have any pseudoconstant quals to deal with. Then, if
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index 73da0c2..274fea0 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -973,7 +973,6 @@ deconstruct_recurse(PlannerInfo *root, Node *jtnode, bool below_outer_join,
*postponed_qual_list = lappend(*postponed_qual_list, pq);
}
}
- /* list_concat is nondestructive of its second argument */
my_quals = list_concat(my_quals, (List *) j->quals);
/*
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 36fefd9..6de182c 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -3567,10 +3567,6 @@ reorder_grouping_sets(List *groupingsets, List *sortclause)
}
}
- /*
- * Safe to use list_concat (which shares cells of the second arg)
- * because we know that new_elems does not share cells with anything.
- */
previous = list_concat(previous, new_elems);
gs->set = list_copy(previous);
@@ -4282,8 +4278,8 @@ consider_groupingsets_paths(PlannerInfo *root,
*/
if (!rollup->hashable)
return;
- else
- sets_data = list_concat(sets_data, list_copy(rollup->gsets_data));
+
+ sets_data = list_concat(sets_data, rollup->gsets_data);
}
foreach(lc, sets_data)
{
@@ -4468,7 +4464,7 @@ consider_groupingsets_paths(PlannerInfo *root,
{
if (bms_is_member(i, hash_items))
hash_sets = list_concat(hash_sets,
- list_copy(rollup->gsets_data));
+ rollup->gsets_data);
else
rollups = lappend(rollups, rollup);
++i;
@@ -5637,8 +5633,7 @@ make_pathkeys_for_window(PlannerInfo *root, WindowClause *wc,
errdetail("Window ordering columns must be of sortable datatypes.")));
/* Okay, make the combined pathkeys */
- window_sortclauses = list_concat(list_copy(wc->partitionClause),
- list_copy(wc->orderClause));
+ window_sortclauses = list_concat_copy(wc->partitionClause, wc->orderClause);
window_pathkeys = make_pathkeys_for_sortclauses(root,
window_sortclauses,
tlist);
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index dc11f09..85228e9 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -889,7 +889,7 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
splan->resultRelIndex = list_length(root->glob->resultRelations);
root->glob->resultRelations =
list_concat(root->glob->resultRelations,
- list_copy(splan->resultRelations));
+ splan->resultRelations);
/*
* If the main target relation is a partitioned table, also
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index 4fbc03f..3c0ddd6 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -2507,7 +2507,6 @@ reduce_outer_joins_pass2(Node *jtnode,
pass_nonnullable_rels = find_nonnullable_rels(f->quals);
pass_nonnullable_rels = bms_add_members(pass_nonnullable_rels,
nonnullable_rels);
- /* NB: we rely on list_concat to not damage its second argument */
pass_nonnullable_vars = find_nonnullable_vars(f->quals);
pass_nonnullable_vars = list_concat(pass_nonnullable_vars,
nonnullable_vars);
diff --git a/src/backend/optimizer/prep/prepqual.c b/src/backend/optimizer/prep/prepqual.c
index e9a9497..ee91957 100644
--- a/src/backend/optimizer/prep/prepqual.c
+++ b/src/backend/optimizer/prep/prepqual.c
@@ -328,12 +328,6 @@ pull_ands(List *andlist)
{
Node *subexpr = (Node *) lfirst(arg);
- /*
- * Note: we can destructively concat the subexpression's arglist
- * because we know the recursive invocation of pull_ands will have
- * built a new arglist not shared with any other expr. Otherwise we'd
- * need a list_copy here.
- */
if (is_andclause(subexpr))
out_list = list_concat(out_list,
pull_ands(((BoolExpr *) subexpr)->args));
@@ -360,12 +354,6 @@ pull_ors(List *orlist)
{
Node *subexpr = (Node *) lfirst(arg);
- /*
- * Note: we can destructively concat the subexpression's arglist
- * because we know the recursive invocation of pull_ors will have
- * built a new arglist not shared with any other expr. Otherwise we'd
- * need a list_copy here.
- */
if (is_orclause(subexpr))
out_list = list_concat(out_list,
pull_ors(((BoolExpr *) subexpr)->args));
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 99dbf8d..1e465b2 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -1009,8 +1009,8 @@ max_parallel_hazard_walker(Node *node, max_parallel_hazard_context *context)
max_parallel_hazard_test(PROPARALLEL_RESTRICTED, context))
return true;
save_safe_param_ids = context->safe_param_ids;
- context->safe_param_ids = list_concat(list_copy(subplan->paramIds),
- context->safe_param_ids);
+ context->safe_param_ids = list_concat_copy(context->safe_param_ids,
+ subplan->paramIds);
if (max_parallel_hazard_walker(subplan->testexpr, context))
return true; /* no need to restore safe_param_ids */
list_free(context->safe_param_ids);
@@ -3697,18 +3697,12 @@ simplify_or_arguments(List *args,
/* flatten nested ORs as per above comment */
if (is_orclause(arg))
{
- List *subargs = list_copy(((BoolExpr *) arg)->args);
+ List *subargs = ((BoolExpr *) arg)->args;
+ List *oldlist = unprocessed_args;
- /* overly tense code to avoid leaking unused list header */
- if (!unprocessed_args)
- unprocessed_args = subargs;
- else
- {
- List *oldhdr = unprocessed_args;
-
- unprocessed_args = list_concat(subargs, unprocessed_args);
- pfree(oldhdr);
- }
+ unprocessed_args = list_concat_copy(subargs, unprocessed_args);
+ /* perhaps-overly-tense code to avoid leaking old lists */
+ list_free(oldlist);
continue;
}
@@ -3718,14 +3712,14 @@ simplify_or_arguments(List *args,
/*
* It is unlikely but not impossible for simplification of a non-OR
* clause to produce an OR. Recheck, but don't be too tense about it
- * since it's not a mainstream case. In particular we don't worry
- * about const-simplifying the input twice.
+ * since it's not a mainstream case. In particular we don't worry
+ * about const-simplifying the input twice, nor about list leakage.
*/
if (is_orclause(arg))
{
- List *subargs = list_copy(((BoolExpr *) arg)->args);
+ List *subargs = ((BoolExpr *) arg)->args;
- unprocessed_args = list_concat(subargs, unprocessed_args);
+ unprocessed_args = list_concat_copy(subargs, unprocessed_args);
continue;
}
@@ -3799,18 +3793,12 @@ simplify_and_arguments(List *args,
/* flatten nested ANDs as per above comment */
if (is_andclause(arg))
{
- List *subargs = list_copy(((BoolExpr *) arg)->args);
+ List *subargs = ((BoolExpr *) arg)->args;
+ List *oldlist = unprocessed_args;
- /* overly tense code to avoid leaking unused list header */
- if (!unprocessed_args)
- unprocessed_args = subargs;
- else
- {
- List *oldhdr = unprocessed_args;
-
- unprocessed_args = list_concat(subargs, unprocessed_args);
- pfree(oldhdr);
- }
+ unprocessed_args = list_concat_copy(subargs, unprocessed_args);
+ /* perhaps-overly-tense code to avoid leaking old lists */
+ list_free(oldlist);
continue;
}
@@ -3820,14 +3808,14 @@ simplify_and_arguments(List *args,
/*
* It is unlikely but not impossible for simplification of a non-AND
* clause to produce an AND. Recheck, but don't be too tense about it
- * since it's not a mainstream case. In particular we don't worry
- * about const-simplifying the input twice.
+ * since it's not a mainstream case. In particular we don't worry
+ * about const-simplifying the input twice, nor about list leakage.
*/
if (is_andclause(arg))
{
- List *subargs = list_copy(((BoolExpr *) arg)->args);
+ List *subargs = ((BoolExpr *) arg)->args;
- unprocessed_args = list_concat(subargs, unprocessed_args);
+ unprocessed_args = list_concat_copy(subargs, unprocessed_args);
continue;
}
@@ -4188,7 +4176,7 @@ add_function_defaults(List *args, HeapTuple func_tuple)
defaults = list_copy_tail(defaults, ndelete);
/* And form the combined argument list, not modifying the input list */
- return list_concat(list_copy(args), defaults);
+ return list_concat_copy(args, defaults);
}
/*
diff --git a/src/backend/optimizer/util/orclauses.c b/src/backend/optimizer/util/orclauses.c
index 18ebc51..412a396 100644
--- a/src/backend/optimizer/util/orclauses.c
+++ b/src/backend/optimizer/util/orclauses.c
@@ -236,7 +236,7 @@ extract_or_clause(RestrictInfo *or_rinfo, RelOptInfo *rel)
subclause = (Node *) make_ands_explicit(subclauses);
if (is_orclause(subclause))
clauselist = list_concat(clauselist,
- list_copy(((BoolExpr *) subclause)->args));
+ ((BoolExpr *) subclause)->args);
else
clauselist = lappend(clauselist, subclause);
}
diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c
index 37d228c..5a4d696 100644
--- a/src/backend/optimizer/util/relnode.c
+++ b/src/backend/optimizer/util/relnode.c
@@ -1715,43 +1715,39 @@ build_joinrel_partition_info(RelOptInfo *joinrel, RelOptInfo *outer_rel,
*/
for (cnt = 0; cnt < partnatts; cnt++)
{
- List *outer_expr;
- List *outer_null_expr;
- List *inner_expr;
- List *inner_null_expr;
+ /* mark these const to enforce that we copy them properly */
+ const List *outer_expr = outer_rel->partexprs[cnt];
+ const List *outer_null_expr = outer_rel->nullable_partexprs[cnt];
+ const List *inner_expr = inner_rel->partexprs[cnt];
+ const List *inner_null_expr = inner_rel->nullable_partexprs[cnt];
List *partexpr = NIL;
List *nullable_partexpr = NIL;
- outer_expr = list_copy(outer_rel->partexprs[cnt]);
- outer_null_expr = list_copy(outer_rel->nullable_partexprs[cnt]);
- inner_expr = list_copy(inner_rel->partexprs[cnt]);
- inner_null_expr = list_copy(inner_rel->nullable_partexprs[cnt]);
-
switch (jointype)
{
case JOIN_INNER:
- partexpr = list_concat(outer_expr, inner_expr);
- nullable_partexpr = list_concat(outer_null_expr,
- inner_null_expr);
+ partexpr = list_concat_copy(outer_expr, inner_expr);
+ nullable_partexpr = list_concat_copy(outer_null_expr,
+ inner_null_expr);
break;
case JOIN_SEMI:
case JOIN_ANTI:
- partexpr = outer_expr;
- nullable_partexpr = outer_null_expr;
+ partexpr = list_copy(outer_expr);
+ nullable_partexpr = list_copy(outer_null_expr);
break;
case JOIN_LEFT:
- partexpr = outer_expr;
- nullable_partexpr = list_concat(inner_expr,
- outer_null_expr);
+ partexpr = list_copy(outer_expr);
+ nullable_partexpr = list_concat_copy(inner_expr,
+ outer_null_expr);
nullable_partexpr = list_concat(nullable_partexpr,
inner_null_expr);
break;
case JOIN_FULL:
- nullable_partexpr = list_concat(outer_expr,
- inner_expr);
+ nullable_partexpr = list_concat_copy(outer_expr,
+ inner_expr);
nullable_partexpr = list_concat(nullable_partexpr,
outer_null_expr);
nullable_partexpr = list_concat(nullable_partexpr,
diff --git a/src/backend/optimizer/util/tlist.c b/src/backend/optimizer/util/tlist.c
index 7ccb10e..d75796a 100644
--- a/src/backend/optimizer/util/tlist.c
+++ b/src/backend/optimizer/util/tlist.c
@@ -665,9 +665,8 @@ make_tlist_from_pathtarget(PathTarget *target)
* copy_pathtarget
* Copy a PathTarget.
*
- * The new PathTarget has its own List cells, but shares the underlying
- * target expression trees with the old one. We duplicate the List cells
- * so that items can be added to one target without damaging the other.
+ * The new PathTarget has its own exprs List, but shares the underlying
+ * target expression trees with the old one.
*/
PathTarget *
copy_pathtarget(PathTarget *src)
diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c
index 354030e..f418c61 100644
--- a/src/backend/parser/parse_agg.c
+++ b/src/backend/parser/parse_agg.c
@@ -1649,9 +1649,8 @@ expand_groupingset_node(GroupingSet *gs)
Assert(gs_current->kind == GROUPING_SET_SIMPLE);
- current_result
- = list_concat(current_result,
- list_copy(gs_current->content));
+ current_result = list_concat(current_result,
+ gs_current->content);
/* If we are done with making the current group, break */
if (--i == 0)
@@ -1691,11 +1690,8 @@ expand_groupingset_node(GroupingSet *gs)
Assert(gs_current->kind == GROUPING_SET_SIMPLE);
if (mask & i)
- {
- current_result
- = list_concat(current_result,
- list_copy(gs_current->content));
- }
+ current_result = list_concat(current_result,
+ gs_current->content);
mask <<= 1;
}
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 2a6b2ff..260ccd4 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -1214,9 +1214,6 @@ transformFromClauseItem(ParseState *pstate, Node *n,
*
* Notice that we don't require the merged namespace list to be
* conflict-free. See the comments for scanNameSpaceForRefname().
- *
- * NB: this coding relies on the fact that list_concat is not
- * destructive to its second argument.
*/
lateral_ok = (j->jointype == JOIN_INNER || j->jointype == JOIN_LEFT);
setNamespaceLateralState(l_namespace, true, lateral_ok);
@@ -2116,9 +2113,7 @@ flatten_grouping_sets(Node *expr, bool toplevel, bool *hasGroupingSets)
if (IsA(n1, GroupingSet) &&
((GroupingSet *) n1)->kind == GROUPING_SET_SETS)
- {
result_set = list_concat(result_set, (List *) n2);
- }
else
result_set = lappend(result_set, n2);
}
diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index 6d3751d..b8efd6b 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -642,8 +642,8 @@ gen_partprune_steps(RelOptInfo *rel, List *clauses, PartClauseTarget target,
if (rel->relid != 1)
ChangeVarNodes((Node *) partqual, 1, rel->relid, 0);
- /* Use list_copy to avoid modifying the passed-in List */
- clauses = list_concat(list_copy(clauses), partqual);
+ /* Make a copy to avoid modifying the passed-in List */
+ clauses = list_concat_copy(clauses, partqual);
}
/* Down into the rabbit-hole. */
@@ -1471,7 +1471,7 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
pc->keyno,
NULL,
prefix);
- opsteps = list_concat(opsteps, list_copy(pc_steps));
+ opsteps = list_concat(opsteps, pc_steps);
}
}
break;
@@ -1542,7 +1542,7 @@ gen_prune_steps_from_opexps(GeneratePruningStepsContext *context,
pc->keyno,
nullkeys,
prefix);
- opsteps = list_concat(opsteps, list_copy(pc_steps));
+ opsteps = list_concat(opsteps, pc_steps);
}
}
break;
diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c
index 79c7c13..a21f7d3 100644
--- a/src/backend/replication/syncrep.c
+++ b/src/backend/replication/syncrep.c
@@ -865,8 +865,6 @@ SyncRepGetSyncStandbysPriority(bool *am_sync)
*/
if (list_length(result) + list_length(pending) <= SyncRepConfig->num_sync)
{
- bool needfree = (result != NIL && pending != NIL);
-
/*
* Set *am_sync to true if this walsender is in the pending list
* because all pending standbys are considered as sync.
@@ -875,8 +873,7 @@ SyncRepGetSyncStandbysPriority(bool *am_sync)
*am_sync = am_in_pending;
result = list_concat(result, pending);
- if (needfree)
- pfree(pending);
+ list_free(pending);
return result;
}
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 93b6784..8249f71 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -1041,11 +1041,11 @@ process_matched_tle(TargetEntry *src_tle,
/* combine the two */
memcpy(fstore, prior_expr, sizeof(FieldStore));
fstore->newvals =
- list_concat(list_copy(((FieldStore *) prior_expr)->newvals),
- list_copy(((FieldStore *) src_expr)->newvals));
+ list_concat_copy(((FieldStore *) prior_expr)->newvals,
+ ((FieldStore *) src_expr)->newvals);
fstore->fieldnums =
- list_concat(list_copy(((FieldStore *) prior_expr)->fieldnums),
- list_copy(((FieldStore *) src_expr)->fieldnums));
+ list_concat_copy(((FieldStore *) prior_expr)->fieldnums,
+ ((FieldStore *) src_expr)->fieldnums);
}
else
{
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 4ca0ed2..5869ba5 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -10089,7 +10089,7 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
RangeTblFunction *rtfunc = (RangeTblFunction *) lfirst(lc);
List *args = ((FuncExpr *) rtfunc->funcexpr)->args;
- allargs = list_concat(allargs, list_copy(args));
+ allargs = list_concat(allargs, args);
}
appendStringInfoString(buf, "UNNEST(");
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 7eba59e..1710129 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -5768,7 +5768,6 @@ add_predicate_to_index_quals(IndexOptInfo *index, List *indexQuals)
if (!predicate_implied_by(oneQual, indexQuals, false))
predExtraQuals = list_concat(predExtraQuals, oneQual);
}
- /* list_concat avoids modifying the passed-in indexQuals list */
return list_concat(predExtraQuals, indexQuals);
}
diff --git a/src/include/nodes/pg_list.h b/src/include/nodes/pg_list.h
index 1463408..409d840 100644
--- a/src/include/nodes/pg_list.h
+++ b/src/include/nodes/pg_list.h
@@ -519,6 +519,8 @@ extern List *lcons_int(int datum, List *list);
extern List *lcons_oid(Oid datum, List *list);
extern List *list_concat(List *list1, const List *list2);
+extern List *list_concat_copy(const List *list1, const List *list2);
+
extern List *list_truncate(List *list, int new_size);
extern bool list_member(const List *list, const void *datum);
pgsql-hackers by date: