On Mon, Jun 1, 2026 at 12:56 PM Peter Smith <smithpb2250@gmail.com> wrote:
>
> Hi Nisha.
>
> Some review comments for patch v8-0003.
>
Thanks for the
> ======
> src/backend/commands/publicationcmds.c
>
> AlterPublicationSchemaExceptTables:
>
> 1.
> + /*
> + * EXCEPT is not meaningful with DROP: dropping a schema from a
> + * publication already removes all its except entries via cascade, and
> + * there is no sensible interpretation of "drop only the except entry but
> + * keep the schema".
> + */
>
> Is that backwards? I think you mean :
>
> SUGGESTION
> * Dropping a schema from a publication removes all its EXCEPT entries via
> * cascade. The concept of "drop all schema tables from the publication EXCEPT
> * these ones" is not supported.
Fixed as suggested in v9.
> ======
> src/bin/pg_dump/t/002_pg_dump.pl
>
> 2.
> On Sat, May 30, 2026 at 2:32 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> ...
> > I don't see any existing "..test continues..." pattern, so I changed it as -
> > 'CREATE PUBLICATION pub11 - ADD TABLES IN SCHEMA EXCEPT dump'
> >
> > Thoughts?
>
> I've since found that there is a way to combine multiple regex within
> a single test. Doing it like below is a cleaner way to write these
> multi-statement tests.
>
> SUGGESTION (note /xms instead of /xm)
> 'CREATE PUBLICATION pub11' => {
> create_order => 50,
> create_sql =>
> 'CREATE PUBLICATION pub11 FOR TABLES IN SCHEMA dump_test
> EXCEPT (TABLE test_table);',
> regexp => qr/^
> \QCREATE PUBLICATION pub11 WITH (publish = 'insert,
> update, delete, truncate');\E
> .*?
> \QALTER PUBLICATION pub11 ADD TABLES IN SCHEMA dump_test
> EXCEPT (TABLE ONLY test_table);\E
> /xms,
> like => { %full_runs, section_post_data => 1, },
> },
>
> (ditto for the pub12 test)
>
Thanks for the suggestion. I've updated the test accordingly in v9.
--
Thanks,
Nisha