On Thu, 26 Feb 2026 at 11:42, shveta malik <shveta.malik@gmail.com> wrote:
>
> On Wed, Feb 25, 2026 at 10:43 PM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
> >
> > Hi Shveta, Ashutosh, Chao-san,
> >
> > Thanks for reviewing the patch.
> > I have addressed the above comments and the comments in [1] and [2].
> > Attached is the latest v50 version.
> >
>
> Thank You Shlok. Please find a few comments on v50-001.
>
> 1)
> -check_publication_add_relation(Relation targetrel)
> +check_publication_add_relation(PublicationRelInfo *pri, bool puballtables)
>
> Why do we need argument puballtables? I think we can give partition
> related error even without checking 'puballtables', like we are giving
> for temp and unlogged table error in EXCEPT list without checking
> puballtables. Or let me know if I am missing the point here?
This is required as this is a common function used by both all tables
and table publication, since we allow child tables in case of table
publication, alltables check is required.
> 2)
> publication_has_any_except_table(): Shall we optimize it:
> a) if it is not all-table pub, return false there itself.
> b) if it is all table, proceed with fetching tuples. At first tuple we
> can break the loop irrespective of check 'pubrel->prexcept', as there
> is no other feasibility. But we shall Assert (pubrel->prexcept) .
> Thoughts?
As Amit is also not in favor of this at [1], I'm not doing any change for this.
Rest of the comments are addressed in the attached v51 version.
Regarding the 12 comment from [2]:
> 12) pg_dump dumped "ONLY" along with ROOT name in EXCEPT. I think it
> should be avoided.
> pg_dump output:
> CREATE PUBLICATION pub4 FOR ALL TABLES EXCEPT TABLE (ONLY
> public.tab_root) WITH (publish = 'insert, update, delete, truncate');
I felt this is ok, we dump similarly in case of table publications too
like for the below publication:
create publication pub1 for table tab_root ;
Dump adds only here too:
--
-- Name: pub1 tab_root; Type: PUBLICATION TABLE; Schema: public; Owner: vignesh
--
ALTER PUBLICATION pub1 ADD TABLE ONLY public.tab_root;
Rest of the comments from [2] have been addressed.
[1] - https://www.postgresql.org/message-id/CAA4eK1%2BV0-HaxqiEJShTOh5%2BSGw1p4kurK327n8w3WwLV9DD4Q%40mail.gmail.com
[2] - https://www.postgresql.org/message-id/CAJpy0uDRv%3DVuG439QTVaX6pgBSvpzAQd5Px9qH%2BE-XAnwpMuDQ%40mail.gmail.com
Regards,
Vignesh
