Re: Skipping schema changes in publication

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

thanks
Shveta