Re: Logical Replication - revisit `is_table_publication` function implementation

On Thu, Aug 20, 2026 at 11:40 AM Bingshuai Li <lucian1412(at)outlook(dot)com> wrote:
>
> Hi Peter,
>
> Thanks for the quick v3 — the doc comments and the merged coexistence
> sentences all look good.
>
> One point from my review seems to have been dropped, though. The merged
> comment in is_table_publication() now reads:
>
> /*
> * FOR TABLE cannot be used with FOR ALL TABLES or FOR ALL SEQUENCES.
> */
>
> That states the grammar fact, but not why skipping the
> pg_publication_rel scan is safe: a FOR ALL TABLES publication can still
> have EXCEPT rows there, and returning false is correct precisely because
> those are the only rows it can have. Would you consider keeping one more
> sentence to that effect, e.g.
>
> /*
> * FOR TABLE cannot be used with FOR ALL TABLES or FOR ALL SEQUENCES.
> * A FOR ALL TABLES publication can have only EXCEPT entries in
> * pg_publication_rel, so it never counts as a table publication.
> */
>

The name of the function is "is_table_publication". The purpose is in
the name. Just return T/F if there is "FOR TABLE" clause for the
publication; nothing more.
e.g., if the function was called "is_table_in_pg_publication_rel",
then I would want to explain about EXCEPT.

Since "FOR ALL TABLES" is already mutually exclusive from "FOR TABLE"
I didn't see any need to explain about "FOR ALL TABLES EXCEPT".

Was your review AI-based? It appeared overly concerned about this detail.

Anyway, I am marking this as "ready for committer" so a committer can
decide on this point.

======
Kind Regards,
Peter Smith.
Fujitsu Australia