Re: Distinguish publication exclusions in object addresses

On Fri, 7 Aug 2026 at 11:42, Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> Some review comments for v1.
>
> ======
> src/backend/catalog/objectaddress.c
>
> 1.
> + {
> + "publication exclusion", OBJECT_PUBLICATION_REL
> + },
>
> I wonder if it's better to call this "publication excluded relation".
>
> e.g. One day there might be the ability to do "FOR ALL TABLES EXCEPT
> (SCHEMA s)", but then "publication exclusion" would not know whether
> you are referring to relations or schemas.

Vignesh - Agree on this. There is an exception sequence patch already
in discussion.

> ~~~
>
> get_object_address_publication_rel:
>
> 2.
> - if (!OidIsValid(address.objectId))
> + if (OidIsValid(address.objectId) &&
> + pubrel_is_exclusion == isPublicationRelationExcept(address.objectId,
> + missing_ok))
> {
> - if (!missing_ok)
> + *relp = relation;
> + return address;
> + }
>
> 2a.
> I'm not clear on why we are passing `missing_ok` here. If the
> OidIsValid(address.objectId) is true, then AFAICT the row *must* by
> definition exist in the pg_publication_rel, in which case we should
> never pass `missing_ok` as true.

This comment is not applicable anymore as isPublicationRelationExcept
is no more called from get_object_address_publication_rel

> ~
>
> 2b.
> Actually, I found that the isPublicationRelationExcept call in the
> condition made the logic hard to understand. Can it be expanded out
> and code kept more like the original? Maybe something like below?
>
> SUGGESTION
> if (!OidIsvalid(address.objectId))
> {
> if (!missing_ok)
> {
> if (pub_is_exclusion)
> ereport ...
> else
> ereport
> }
> relation_close(relation, AccessShareLock);
> return address;
> }
> else
> {
> /* Found row in pg_publication_rel */
>
> /* Treat a prexcept mismatch as not found. */
> if (pubrel_is_exclusion !=
> isPublicationRelationExcept(address.objectId, false))
> {
> address.objectId = InvalidOid;
> relation_close(relation, AccessShareLock);
> return address;
> }
> }
>
> *relp = relation;
> return address;

This part of code has been changed to handle another comment of
Shveta, let me know if you feel if it requires any other change.

> ~~~
>
> isPublicationRelationExcept:
>
> 3.
> +/*
> + * Return whether an existing pg_publication_rel entry represents a publication
> + * EXCEPT entry.
> + */
> +static bool
> +isPublicationRelationExcept(Oid pubreloid, bool missing_ok)
>
> The `missing_ok` parameter means the function handles both missing and
> present entries, so perhaps you don't need to say "an existing" in
> that function comment.

Rest of the comments were handled.

The attached patches have the changes for the same.
v1-0001-Distinguish-publication-exclusions-in-object-addr.patch is the
same Fujii Masao-san's patch from [2]. The comment fixes are present
in v1-0002-Review-comment-fixes.patch which is a top-up patch on top
of Fujii Masao-san's patch.

@Fujii Masao -san Please merge the changes if you are ok with the changes.

[1] - https://www.postgresql.org/message-id/CANhcyEVSXyQkvmrsOWPdQqnm2J3GMyQQrKhyCJiBQzqs6AvSow%40mail.gmail.com
[2] - https://www.postgresql.org/message-id/CAHGQGwHfESBexa7fq99EvFCf31av=O9h9udnw22ymxmm6LMZzw@mail.gmail.com

Regards,
Vignesh