On 9/2/2025 18:41, Alexander Korotkov wrote:
Regarding adjust_relid_set() and replace_relid(). I think they are now strictly equivalent, except for the case then old relid is given and not found. In this case adjust_relid_set() returns the original relids while replace_relid() returns a copy. The behavior of adjust_relid_set() appears more desirable as we don't need extra copying when no modification is done. So, I've replaced all replace_relid() with adjust_relid_set().
Ok, I glanced into it, and it makes sense to merge these routines.I think the comment to adjust_relid_set() should be arranged, too. See the attachment for a short variant of such modification.
Also, I did some grammar correction to your new comment in tests.
Thanks! -- regards, Andrei Lepikhov
diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c index 6d56b65a4b..38c3e4e225 100644 --- a/src/backend/rewrite/rewriteManip.c +++ b/src/backend/rewrite/rewriteManip.c @@ -794,12 +794,10 @@ ChangeVarNodes(Node *node, int rt_index, int new_index, int sublevels_up) } /* - * Substitute newrelid for oldrelid in a Relid set - * + * Remove oldrelid from a Relid set and replace it with newrelid in case it is + * not a special varno. * Note: some extensions may pass a special varno such as INDEX_VAR for * oldrelid. bms_is_member won't like that, but we should tolerate it. - * (Perhaps newrelid could also be a special varno, but there had better - * not be a reason to inject that into a nullingrels or phrels set.) */ Relids adjust_relid_set(Relids relids, int oldrelid, int newrelid)
