public inbox for [email protected]  
help / color / mirror / Atom feed
From: Peter Smith <[email protected]>
To: Shlok Kyal <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Zhijie Hou (Fujitsu) <[email protected]>
Cc: YeXiu <[email protected]>
Cc: Ian Lawrence Barwick <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Skipping schema changes in publication
Date: Thu, 20 Nov 2025 12:55:16 +1100
Message-ID: <CAHut+Pv4d9EAjDQiOHiu2BrYP3ZA-oJgsgGZdygBaZnWDR7sDA@mail.gmail.com> (raw)
In-Reply-To: <CANhcyEXCKPCAdoqBLAhxt64Nwf+7T52dd8daE3qvhBNTvro13Q@mail.gmail.com>
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>
	<CAHut+PsXP_61ZXuVOx5u9FZGK3oH4taaA59oOzgqyygZx8ezWw@mail.gmail.com>
	<CANhcyEU+aPu6iAH2cTA0cDtn3pd6c_njBONCt3FubYZoEEnm8Q@mail.gmail.com>
	<CAHut+Pv2P6dJ7hZj_fmzN+=xzjvpOpgkAJvDZg3TD2xpvmY1NQ@mail.gmail.com>
	<CANhcyEW2LK4diNeCG862DE40yQoV3VAgf59kXUq2TuR8fnw5vQ@mail.gmail.com>
	<CAHut+PuSHScrODVGCM7P53Mv1HE2N6ThzkH4+gQ1eFXVeD-OCA@mail.gmail.com>
	<CANhcyEUtYV-9ujtxLasnxN_peT+3LuZjcRx1xUECh1CCmANB8w@mail.gmail.com>
	<CAHut+PuviFA6C7qps=+kDYfe3P99as8NCjbR=SYxoi0o96ipoA@mail.gmail.com>
	<CANhcyEXkeg3sjkS3DS9yU1ckz4ozUBNZ+RmrWaRNSSVCR8RquA@mail.gmail.com>
	<CAHut+PsHavMy_KJ0MwR9J6q0BTTty54TxS-KBZc7X6tb4u7rfA@mail.gmail.com>
	<CANhcyEU=k4+0BqOu25N76g738XKUwfLGGdf8e+ssGiRKHC4RwQ@mail.gmail.com>
	<CAHut+Ptdi8txJbMc+5Sp2uB4QLGOp-rZdkBbXyhAkxim0iAhBQ@mail.gmail.com>
	<CANhcyEW+uJB_bvQLEaZCgoRTc1=i+QnrPPHxZ2=0SBSCyj9pkg@mail.gmail.com>
	<CAHut+Ptq0-tMYUOvG3yR34AvuEzR9vUH=muqV_=uEO3zCuA6rA@mail.gmail.com>
	<CANhcyEUEMWSkTfGc7Q3B+UiOzSiOmOGLgK-+C5DXwtCGOnDBhg@mail.gmail.com>
	<CAHut+PuAgSDr3kbSxPMYEyCeGiJ5hgaT1JUvuiYPRT=Q+--O-Q@mail.gmail.com>
	<CANhcyEU_uuiKMRrd_E_DeYsyCvwY_u995E-Do3d66J7tQnzdzw@mail.gmail.com>
	<CANhcyEWK9sj9UY4uaV36Q8qxUv=8Joch0o98RCN5U3xLjUxAag@mail.gmail.com>
	<CALDaNm32XQDR4qsOhPQeophVbZ8r+ShJSSssoVfdPcwG6joPHQ@mail.gmail.com>
	<CANhcyEVt2CBnG7MOktaPPV4rYapHR-VHe5=qoziTZh1L9SVc6w@mail.gmail.com>
	<CAHut+PtGu2j72yV_as_TVKfWr3ctd18svReGEx3xa=q5BtKyKA@mail.gmail.com>
	<CANhcyEUh9ki36VTXyYf8UqUrfLX9ZhfP_f2LjpvvycgqWLQqqQ@mail.gmail.com>
	<CAHut+Pv96u0VciuSx2B99jDHEvn7svVJynCmw-qYb=z4Kc2knA@mail.gmail.com>
	<CANhcyEWGiWwGt1-v6d9bCAae9Np7cNPt+iRV9PXBZ0z=75XEVw@mail.gmail.com>
	<CANhcyEWAQHtUfgNPA2m-+okEh7pXaK5irBm+yzyNVJXL2LUTXw@mail.gmail.com>
	<CAHut+PuzcKXheZwgNvDJkwK5txd1kzNRxCmcJcbr=_9mGHjKtA@mail.gmail.com>
	<CANhcyEXCKPCAdoqBLAhxt64Nwf+7T52dd8daE3qvhBNTvro13Q@mail.gmail.com>

Hi Shlok,

Some review comments for patch v28-0001.

======
src/backend/commands/publicationcmds.c

AlterPublicationReset:
1.
+ values[Anum_pg_publication_puballtables - 1] =
BoolGetDatum(PUB_DEFAULT_ALL_TABLES);
+ replaces[Anum_pg_publication_puballtables - 1] = true;
+
+ values[Anum_pg_publication_puballsequences - 1] =
BoolGetDatum(PUB_DEFAULT_ALL_SEQUENCES);
+ replaces[Anum_pg_publication_puballsequences - 1] = true;

The PUB_DEFAULT_xxx made sense for all the publication parameters, but
these are just flags that say if the ALL TABLES or ALL SEQUENCES
clauses are present in the command, so I'm not sure that "default"
macros are appropriate here.

e.g. it seems better to reset those using hardwired booleans:

values[Anum_pg_publication_puballtables - 1] = BoolGetDatum(false);
values[Anum_pg_publication_puballsequences - 1] = BoolGetDatum(false);

(This would also be consistent with what the function comment is saying)

======
src/include/catalog/pg_publication.h

2.
+/* default values for flags and publication parameters */
+#define PUB_DEFAULT_ACTION_INSERT true
+#define PUB_DEFAULT_ACTION_UPDATE true
+#define PUB_DEFAULT_ACTION_DELETE true
+#define PUB_DEFAULT_ACTION_TRUNCATE true
+#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

As in the previous comment #1, I don't think we should define
PUB_DEFAULT_ALL_TABLES and PUB_DEFAULT_ALL_SEQUENCES. Also, change the
comment to remove the words "flags and".

======
src/test/regress/sql/publication.sql

3.
I don't think the parameter names should be written in uppercase
because that's not how they normally appear in the docs and examples.

~~~

4.
+-- Verify that 'ALL TABLES', 'ALL SEQUENCES' flag is reset

typo: /flag is reset/flags are reset/

~~~

5.
+-- Verify that associated tables, schemas and the publication parameters
+-- 'PUBLISH', 'PUBLISH_VIA_PARTITION_ROOT', and 'PUBLISH_GENERATED_COLUMNS'
+-- are removed from the publication after RESET

The comment makes it sound like parameters are "removed". Maybe some
rewording is needed.

SUGGESTION
Verify that a publication RESET removes the associated tables and
schemas, and sets default values for publication parameters 'publish',
'publish_via_partition_root', and 'publish_generated_columns'.

======
Kind Regards,
Peter Smith.
Fujitsu Australia





view thread (377+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Skipping schema changes in publication
  In-Reply-To: <CAHut+Pv4d9EAjDQiOHiu2BrYP3ZA-oJgsgGZdygBaZnWDR7sDA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox