Postgres Email Lists

pgsql-hackers

❮

deep copy with mutation?

Jump to comment-1
Robert Haas<robertmhaas@gmail.com>
Apr 29, 2026, 9:22 PM UTC
I spent a lot of time today being stupid before finally understanding
that expressiontreemutator() and querytreemutator() can't be used
to substitute for copyObject() because they deep copy structure only
if it's Node structure; non-nodes like plain old C strings are not
deep-copied. But then I wondered why we don't have a thing that works
that way, because we have stuff like this in a number of places:
```
        parsetree->returningList = copyObject(parsetree->returningList);
        ChangeVarNodes((Node *) parsetree->returningList, rt_index,
                       parsetree->resultRelation, 0);
```
If we had a fully-deeply-copying version of copyObject(), it could
just adjust new Var nodes as it created them, instead of needing a
separate tree walk.
In CreateTriggerFiringOn, we have:
```
        qual = copyObject(whenClause);
        qual = (Node *)
            map_partition_varattnos((List *) qual, PRS2_OLD_VARNO,
                                    childTbl, rel);
        qual = (Node *)
            map_partition_varattnos((List *) qual, PRS2_NEW_VARNO,
                                    childTbl, rel);
```
So a copy and then two consecutive mutation passes.
Or similarly in rewriteTargetView:
```
     tmp_tlist = copyObject(view_targetlist);

     ChangeVarNodes((Node *) tmp_tlist, new_rt_index,
                    new_exclRelIndex, 0);

     parsetree->onConflict = (OnConflictExpr *)
         ReplaceVarsFromTargetList((Node *) parsetree->onConflict,
                                   old_exclRelIndex,
                                   0,
                                   view_rte,
                                   tmp_tlist,
                                   new_rt_index,
                                   REPLACEVARS_REPORT_ERROR,
                                   0,
                                   &parsetree->hasSubLinks);
```
I'm not entirely certain how much this kind of thing matters -- maybe
it's better to have the copy routine be as simple as possible and
accept the cost of walking the whole tree immediately afterwards to
make changes? Perhaps this kind of thing is so cache-friendly that the
mutation pass is really cheap? But it certainly kinda looks
inefficient...
--
Robert Haas
EDB: http://www.enterprisedb.com
View in PostgreSQL Archives →
- Jump to comment-1
  Ashutosh Bapat<ashutosh.bapat.oss@gmail.com>
  Apr 30, 2026, 10:34 AM UTC
  On Thu, Apr 30, 2026 at 2:52 AM Robert Haas <robertmhaas@gmail.com> wrote:
  
  I spent a lot of time today being stupid before finally understanding
  that expressiontreemutator() and querytreemutator() can't be used
  to substitute for copyObject() because they deep copy structure only
  if it's Node structure; non-nodes like plain old C strings are not
  deep-copied. But then I wondered why we don't have a thing that works
  that way, because we have stuff like this in a number of places:
  parsetree->returningList = copyObject(parsetree->returningList);
  ChangeVarNodes((Node *) parsetree->returningList, rt_index,
  parsetree->resultRelation, 0);
  
  If we had a fully-deeply-copying version of copyObject(), it could
  just adjust new Var nodes as it created them, instead of needing a
  separate tree walk.
  
  In CreateTriggerFiringOn, we have:
  
  qual = copyObject(whenClause);
  qual = (Node *)
  mappartitionvarattnos((List *) qual, PRS2OLDVARNO,
  childTbl, rel);
  qual = (Node *)
  mappartitionvarattnos((List *) qual, PRS2NEWVARNO,
  childTbl, rel);
  
  So a copy and then two consecutive mutation passes.
  
  Or similarly in rewriteTargetView:
  
  tmptlist = copyObject(viewtargetlist);
  
  ChangeVarNodes((Node *) tmptlist, newrt_index,
  new_exclRelIndex, 0);
  parsetree->onConflict = (OnConflictExpr *)
  ReplaceVarsFromTargetList((Node *) parsetree->onConflict,
  old_exclRelIndex,
  0,
  view_rte,
  tmp_tlist,
  newrtindex,
  REPLACEVARSREPORTERROR,
  0,
  &parsetree->hasSubLinks);
  
  I'm not entirely certain how much this kind of thing matters -- maybe
  it's better to have the copy routine be as simple as possible and
  accept the cost of walking the whole tree immediately afterwards to
  make changes? Perhaps this kind of thing is so cache-friendly that the
  mutation pass is really cheap? But it certainly kinda looks
  inefficient...
  It looks inefficient in terms of CPU as well as memory since FLATCOPY
  itself does palloc_object() and I see some mutators using
  copyObject(). So we are copying the whole tree node by node and then
  parts of the tree are copied by mutators. I think what's needed is
  instead of FLATCOPY we need a version of copyObject which just creates
  a copy of the node without copying the subtrees but copying the
  structures like C strings and arrays. copyObject() already copies
  arrays and C strings. Add a flag to copyObject to indicate whether we
  want copy subtree or not and then replace FLATCOPY with
  copyObject(node, false) and all existing copyObject as
  copyObject(node, true). When generating _copy* functions, we define
  COPYNODEFIELD as COPYSCALARFIELD. Would that work?
  --
  Best Wishes,
  Ashutosh Bapat
  View in PostgreSQL Archives →