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>
In-Reply-To: 
 <CANhcyEU9jX5wM0R3jHs5-=UFaFehcXC8KMkKow_rphHVben_Nw@mail.gmail.com>
From: shveta malik <shveta.malik@gmail.com>
Date: Mon, 30 Jun 2025 12:28:25 +0530
Message-ID: 
 <CAJpy0uDWRRgg9O8dmbBsS2Lu7SJMMOmsbfYNJG4R8saPVK-EQw@mail.gmail.com>
Subject: Re: Skipping schema changes in publication
To: Shlok Kyal <shlok.kyal.oss@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>,
 shveta malik <shveta.malik@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAJpy0uDWRRgg9O8dmbBsS2Lu7SJMMOmsbfYNJG4R8saPVK-EQw%40mail.gmail.com>
Precedence: bulk

On Fri, Jun 27, 2025 at 3:44=E2=80=AFPM Shlok Kyal <shlok.kyal.oss@gmail.co=
m> wrote:
>
> On Thu, 26 Jun 2025 at 15:27, shveta malik <shveta.malik@gmail.com> wrote=
:
> >
> > On Tue, Jun 24, 2025 at 9:48=E2=80=AFAM Shlok Kyal <shlok.kyal.oss@gmai=
l.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 t=
he
> > +   default state which includes resetting the publication parameters, =
setting
> > +   <literal>ALL TABLES</literal> flag to <literal>false</literal> and
> > +   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.

> > 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 to
> > 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.

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

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 (

postgres=3D# create publication pub4 for
ALL TABLES        TABLE             TABLES IN SCHEMA

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

thanks
Shveta