Re: POC: converting Lists into arrays - Mailing 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);