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...
--
Robert Haas
EDB: http://www.enterprisedb.com