Horton

Re: Skipping schema changes in publication

Shlok Kyal <shlok.kyal.oss@gmail.com>

View thread
From: Shlok Kyal <shlok.kyal.oss@gmail.com>
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>
Date: 2025-06-27T10:14:14Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix miscellaneous issues in EXCEPT publication clause.

  2. Change syntax of EXCEPT TABLE clause in publication commands.

  3. Add support for EXCEPT TABLE in ALTER PUBLICATION.

  4. Allow table exclusions in publications via EXCEPT TABLE.

  5. Add wait_for_subscription_sync for TAP tests.

On Thu, 26 Jun 2025 at 15:27, shveta malik <shveta.malik@gmail.com> wrote:
>
> On Tue, Jun 24, 2025 at 9:48 AM Shlok Kyal <shlok.kyal.oss@gmail.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> 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=# 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=true. 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.

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

[1]: https://www.postgresql.org/message-id/CANhcyEU%2BaPu6iAH2cTA0cDtn3pd6c_njBONCt3FubYZoEEnm8Q%40mail.gmail.com

Thanks and Regards,
Shlok Kyal