On Mon, Jun 1, 2026 at 12:56 PM Peter Smith <[email protected]> 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 <[email protected]> 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


Reply via email to