Re: deep copy with mutation?

On Thu, Apr 30, 2026 at 2:52 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> I spent a lot of time today being stupid before finally understanding
> that expression_tree_mutator() and query_tree_mutator() 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...

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
COPY_NODE_FIELD as COPY_SCALAR_FIELD. Would that work?

--
Best Wishes,
Ashutosh Bapat