Horton

Re: Skipping schema changes in publication

shveta malik <shveta.malik@gmail.com>

View thread
From: shveta malik <shveta.malik@gmail.com>
To: Shlok Kyal <shlok.kyal.oss@gmail.com>
Cc: Peter Smith <smithpb2250@gmail.com>, vignesh C <vignesh21@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.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>
Date: 2025-12-04T04:41:44Z
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 Wed, Dec 3, 2025 at 8:56 AM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Wed, Nov 19, 2025 at 3:34 PM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
> >
> >
> > I have also addressed the comments in [1], [2].
> >
> > [1]: https://www.postgresql.org/message-id/CAHut%2BPtRzCD4-0894cutkU_h8cPNtosN0_oSHn2iAKEfg2ENOQ%40mail.gmail.com
> > [2]: https://www.postgresql.org/message-id/CAHut+PuHn-hohA4OdEJz+Zfukfr41TvMTeTH7NwJ=wg1+94uNA@mail.gmail.com
> >
>
> Thanks for the patch. Please find a few comments on 001:
>
> 1)
> + bool nulls[Natts_pg_publication];
> + bool replaces[Natts_pg_publication];
> + Datum values[Natts_pg_publication];
>
> + memset(values, 0, sizeof(values));
> + memset(nulls, false, sizeof(nulls));
> + memset(replaces, false, sizeof(replaces));
>
> Can we initialize all 3 like below? Then we do not need memset.
> bool nulls[Natts_pg_publication] = {0};
>
> 2)
> AlterPublicationReset():
> Can we reset the columns in same sequence as they are originally
> defined (see pg_publication_d.h), it makes it easy to map when
> comparing/reviewing that we do not have missed any.
>
> 3)
> +/* default values for flags and publication parameters */
> ...
> +#define PUB_DEFAULT_VIA_ROOT false
> +#define PUB_DEFAULT_ALL_TABLES false
> +#define PUB_DEFAULT_ALL_SEQUENCES false
> +#define PUB_DEFAULT_GENCOLS PUBLISH_GENCOLS_NONE
>
> These too we can rearrange as per order in pg_publication_d.h
>
> 4)
> + COMPLETE_WITH("ADD", "DROP", "OWNER TO", "RENAME TO", "RESET", "SET");
>
> Is it supposed to be in alphabetical order? Looking at others, I do
> not think so. If not, then I feel 'SET' first followed by  'RESET'
> seems a more obvious choice to me. See similar (ENABLE followed by
> DISABLE):
> COMPLETE_WITH("CONNECTION", "ENABLE", "DISABLE", "OWNER TO",
>
> 5)
> +DROP TABLE pub_sch1.tbl1;
> +DROP SCHEMA pub_sch1;
>
> We can use 'DROP SCHEMA pub_sch1 CASCADE'. Then we need not to worry
> about dropping the associated table(s) (even if we create more in
> future in this schema).
>

6)
Just before AlterPublicationReset-->LockSchemaList does
SearchSysCacheExists1(), I dropped the schema concurrently, which
resulted in below error:

postgres=# alter publication pub1 reset;
ERROR:  schema with OID 16393 does not exist

I do not think the user should be seeing this error. For
CreatePublication and AlterPublicationSchemas, it makes sense to give
an error when it calls LockSchemaList as user has given the schema
names himself. But here since schemas are internally fetched, I think
it will be logical to skip the
error if it gets dropped concurrently. Thoughts?

If we plan to skip error, we can do so by passing the flag
missing_okay=true. See RangeVarGetRelid and other such functions using
such a flag.

thanks
Shveta