MIME-Version: 1.0
References: 
 <CALDaNm3=JrucjhiiwsYQw5-PGtBHFONa6F7hhWCXMsGvh=tamA@mail.gmail.com>
 <CALj2ACVOzhs+BD+abFV2x4oKJdsDNd6SgsE7r8UjnZDCKGEckA@mail.gmail.com>
 <CAA4eK1K6Kr88d2S0zFdHRMyuoaZeNh+ktU+oigmCuD09_x_-+g@mail.gmail.com>
 <CAHut+PsvC-NezO3MJkdyEz=G1QRje2LntjwhQiEeVbmhOQuBMA@mail.gmail.com>
 <CALDaNm18VH2j8cTqfELHQ=0ZNognbGBhbHPteJenWQC6C2dueQ@mail.gmail.com>
 <CALDaNm0k_0Ccj47wzJzzPFwgQB7w=R5+Q2_nSqYrmMmjhmcRUw@mail.gmail.com>
 <CAHut+Pv_0DwyWoGQNMF+G2AGqMuJTzWQKRtmxaC+=zLTPL-Zkw@mail.gmail.com>
 <CALDaNm2-GJt2HsYTkLqQ=ecm=R-vOBw1=aM_d2EiYbz39x_cTQ@mail.gmail.com>
 <TYCPR01MB8373C3120C2B3112001ED6F1EDCF9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
 <CALDaNm0iZZDB300Dez_97S8G6_RW5QpQ8ef6X3wq8tyK-8wnXQ@mail.gmail.com>
 <CAHut+PtiomM+iyAZHvb2dzfsPvRru266KuBe49hKy2n2h+m_zA@mail.gmail.com>
 <CALDaNm30KDnwX4Czi29fqLb8JBkuwqjbpj9ixwNXXox574NZqQ@mail.gmail.com>
 <CALDaNm1PfKRJsEzbKpyt=v4p3bw+_SzE+LFPsMhR5X+qs+0pPw@mail.gmail.com>
 <TYCPR01MB83730A2F1D6A5303E9C1416AEDD99@TYCPR01MB8373.jpnprd01.prod.outlook.com>
 <CALDaNm0sAU4s1KTLOEWv=rYo5dQK6uFTJn_0FKj3XG1Nv4D-qw@mail.gmail.com>
 <CALDaNm3CLRa95tpas6tEj8x58MUNDShxBNoYS+P8Uq5cryoAOw@mail.gmail.com>
 <CALDaNm0EKC3o=v+F7GneGibuCULGKkBWXmNaVB4GR9HoqD066A@mail.gmail.com>
 <CALDaNm1Z1Rmqj9s6P9ZzmrVA9F_vZ_DwwhYAJmsjqmY6dS3-hA@mail.gmail.com>
 <CAB8KJ=jJGuW=ozKmXZzKDUHZ_-J2ZYGOtJo=i2cnNbSu6=KuYg@mail.gmail.com>
 <CALDaNm1mbFP8fxHU_H1Ex4cT2Aq3n8FE79tq0TO5ThvFnDUYMA@mail.gmail.com>
 <CAB8KJ=jq4RwTs8K7pokmXQwQppP2ChVJLMSAdXaxAX+c1r+mdg@mail.gmail.com>
 <CALDaNm1mJvLni8GODebKBmyegXuZ18bLoG-Pz6H1MCX=vphCYA@mail.gmail.com>
 <CALDaNm3dWZCYDih55qTNAYsjCvYXMFv=46UsDWmfCnXMt3kPCg@mail.gmail.com>
 <CALDaNm1AQZYgT0tALRrkvpP1Q+8+e7vkGCUjQ-jim1C0q3e=zA@mail.gmail.com>
 <CAA4eK1KRdAPC=5=7tQ1GW0cRwD=zaDMi+T4u_k4GxPhPY6e8BQ@mail.gmail.com>
 <OS3PR01MB5718C8BE84B862E7E0CEC29B94BD2@OS3PR01MB5718.jpnprd01.prod.outlook.com>
 <CAA4eK1KYQz7cf46_D=6VkZ4J6Y8vJ88MMi=6zm2TJXDP+V1mLg@mail.gmail.com>
 <CANhcyEXZq4mP5dNgg7u=sMPwvxA4_ZN9U92uZEuzs=0xTu+8Yg@mail.gmail.com>
 <CANhcyEXspT3v5-Tdop9uqQV2HWBvZoN5P0BxXQ6Md6Mr7GXK9A@mail.gmail.com>
 <CAHut+PuiaLOCkiAx9nPnjk6wTbPFvnm9T5svTuKbgwJwTdea8w@mail.gmail.com>
 <CANhcyEV_MePxgftHY65et1WdOAk70M0C7PZ1STPUO8PXHVB1YA@mail.gmail.com>
 <CAHut+Ps0hSNqrjv_jT1AuXxO-CrZue3ixE0jKsxVhtArMrkujQ@mail.gmail.com>
 <CANhcyEXX3viVpYcGHD_fzhf_f6CDQWr2+VBywrJf5zm_XiB4tg@mail.gmail.com>
 <CAJpy0uAamUiZN5LkN9y7_-zT8=OZmW846T8dfgkojFwreV8XOw@mail.gmail.com>
 <CANhcyEU9jX5wM0R3jHs5-=UFaFehcXC8KMkKow_rphHVben_Nw@mail.gmail.com>
 <CAJpy0uDWRRgg9O8dmbBsS2Lu7SJMMOmsbfYNJG4R8saPVK-EQw@mail.gmail.com>
In-Reply-To: 
 <CAJpy0uDWRRgg9O8dmbBsS2Lu7SJMMOmsbfYNJG4R8saPVK-EQw@mail.gmail.com>
From: Shlok Kyal <shlok.kyal.oss@gmail.com>
Date: Sat, 19 Jul 2025 16:14:51 +0530
Message-ID: 
 <CANhcyEWvrPh6zMVXHQ53vY09erraumgmddRwUsA-36dQu4tt6w@mail.gmail.com>
Subject: Re: Skipping schema changes in publication
To: shveta malik <shveta.malik@gmail.com>
Cc: Peter Smith <smithpb2250@gmail.com>,
 Amit Kapila <amit.kapila16@gmail.com>,
	"Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>,
 vignesh C <vignesh21@gmail.com>, YeXiu <1518981153@qq.com>,
	Ian Lawrence Barwick <barwick@gmail.com>,
	Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CANhcyEWvrPh6zMVXHQ53vY09erraumgmddRwUsA-36dQu4tt6w%40mail.gmail.com>
Precedence: bulk

On Mon, 30 Jun 2025 at 12:28, shveta malik <shveta.malik@gmail.com> wrote:
>
> On Fri, Jun 27, 2025 at 3:44=E2=80=AFPM Shlok Kyal <shlok.kyal.oss@gmail.=
com> wrote:
> >
> > On Thu, 26 Jun 2025 at 15:27, shveta malik <shveta.malik@gmail.com> wro=
te:
> > >
> > > On Tue, Jun 24, 2025 at 9:48=E2=80=AFAM Shlok Kyal <shlok.kyal.oss@gm=
ail.com> wrote:
> > > >
> > > >  I have included the changes for
> > > > it in v14-0003 patch.
> > > >
> > > Thanks for the patches. I have reviewed patch001 alone, please find
> > > few comments:
> > >
> > > 1)
> > > +  <para>
> > > +   The <literal>RESET</literal> clause will reset the publication to=
 the
> > > +   default state which includes resetting the publication parameters=
, setting
> > > +   <literal>ALL TABLES</literal> flag to <literal>false</literal> an=
d
> > > +   dropping all relations and schemas that are associated with the
> > > +   publication.
> > >    </para>
> > >
> > > It is misleading, as far as I have understood, we do not drop the
> > > tables or schemas associated with the pub; we just remove those from
> > > the publication's object list. See previous doc:
> > > "The ADD and DROP clauses will add and remove one or more
> > > tables/schemas from the publication"
> > >
> > > Perhaps we want to say the same thing when we speak about the 'drop'
> > > aspect of RESET.
> > I have updated the document.
> >
> > > 2)
> > > AlterPublicationReset():
> > >
> > > + if (!OidIsValid(prid))
> > > + ereport(ERROR,
> > > + (errcode(ERRCODE_UNDEFINED_OBJECT),
> > > + errmsg("relation \"%s\" is not part of the publication",
> > > + get_rel_name(relid))));
> > >
> > > Can you please help me understand which scenario will give this error=
?
> > >
> > > Another question is do we really need this error? IIUC, we generally
> > > give errors if a user has explicitly called out a name of an object
> > > and that object is not found. Example:
> > >
> > > postgres=3D# alter publication pubnew drop table t1,tab2;
> > > ERROR:  relation "t1" is not part of the publication
> > >
> > > While in a few other cases, we pass missing_okay as true and do not
> > > give errors. Please see other callers of performDeletion in
> > > publicationcmds.c itself. There we have usage of missing_okay=3Dtrue.=
 I
> > > have not researched myself, but please analyze the cases where
> > > missing_okay is passed as true to figure out if those match our RESET
> > > case. Try to reproduce if possible and then take a call.
> > I thought about the above point and I also think this check is not
> > required. Also, the function was calling PublicationDropSchemas with
> > missing_ok as false. I have changed it to be true.
> >
>
> Okay. Is there a reason for not using PublicationDropTables() here? We
> have rewritten similar code in the Reset flow.
>
I feel it's better to use the function PublicationDropTables(). Also
proper locking would be required on tables while dropping them from
publication.
Made changes for the same.

> > > 3)
> > > +ALTER PUBLICATION testpub_reset ADD ALL TABLES IN SCHEMA public;
> > > +ERROR:  syntax error at or near "ALL"
> > > +LINE 1: ALTER PUBLICATION testpub_reset ADD ALL TABLES IN SCHEMA pub=
...
> > >
> > > There is a problem in syntax, I think the intention of testcase was t=
o
> > > run this query successfully.
> >
> > I have fixed it.
> >
> > Thanks Shveta for reviewing the patch. I have addressed the comments
> > and posted an updated version v15 in [1].
>
> Thanks for the patches. My review is in progress but please find few
> comments on 002:
>
> 1)
> where exception_object is:
>     [ ONLY ] table_name [ * ]
>
> We have the above in CREATE and ALTER pub docs, but we do not explain
> ONLY with EXCEPT. We do have an explanation of ONLY under 'FOR TABLE'.
> But since 'FOR TABLE' and 'EXCEPT' do not go together, it is somewhat
> difficult to connect the dots and find the information ONLY in the
> context of EXCEPT. We shall have ONLY explained for EXCEPT as well. Or
> we can have ONLY defined in a way that both 'FOR TABLE' and 'EXCEPT'
> can refer to it.
>
In create_publication.sgml, added it under "EXCEPT_TABLE'. In
alter_publication.sgml, modified the document under item 'table_name'
under "<title>Parameters</title>"

> 2)
> We get tab-completion options in this command:
> postgres=3D# create publication pub5 for TABLE tab1 W
> WHERE (  WITH (
>
> Similarly in this command:
> create publication pub5 for TABLES IN SCHEMA s1
>
> But once we have 'EXCEPT TABLE', we do not get further tab-completion
> option like WITH(...)
> create publication pub5 for ALL TABLES EXCEPT TABLE tab1
Fixed

> 3)
> During tab-expansion, 'EXCEPT TABLE' and  'WITH (' in the below
> command looks like they are connecting words. Can the gap be increased
> similar to tab-expansion of next command shown below:
>
> postgres=3D# create publication pub4 for ALL TABLES
> EXCEPT TABLE  WITH (
>
I did not find a place to add any custom space. It is default
behaviour to add 2 spaces between different words. See similar:
postgres=3D# CREATE PUBLICATION pub1 FOR TABLE t1 W
WHERE (  WITH (

> postgres=3D# create publication pub4 for
> ALL TABLES        TABLE             TABLES IN SCHEMA
>
I observed that the space between word is dependent on the length of
longest word. Here the longest word is "TABLES IN SCHEMA". The space
between the words are quite noticeable.

> 4)
> alter_publication.sgml.orig is a left-over in patch002.
Fixed

I have added the changes in the latest v16 patch [1].
[1]: https://www.postgresql.org/message-id/CANhcyEW2LK4diNeCG862DE40yQoV3VA=
gf59kXUq2TuR8fnw5vQ%40mail.gmail.com

Thanks and Regards,
Shlok Kyal