Re: Logical Replication - revisit `is_table_publication` function implementation

From: Bingshuai Li <lucian1412(at)outlook(dot)com>
To: Peter Smith <smithpb2250(at)gmail(dot)com>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Logical Replication - revisit `is_table_publication` function implementation
Date: 2026-08-20 01:40:44
Message-ID: ME0P300MB05689F50B25C44535E037962C6A42@ME0P300MB0568.AUSP300.PROD.OUTLOOK.COM
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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 is_schema_publication() side is fine as written, since
pg_publication_namespace simply has no rows to find in that case.

BTW, v2 -> v3 is comment-only apart from the rebase, so the test results
I posted earlier still stand. With the comment above in place, this gets
my +1.

Regards,
Bingshuai Li

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Xuneng Zhou 2026-08-20 01:58:28 Re: timeout value overflow in wait for lsn
Previous Message Bharath Rupireddy 2026-08-20 01:32:00 Re: [PATCH] Fix NULL dereference in subscription REFRESH on concurrent DROP