Re: Skipping schema changes in publication
| От | Shlok Kyal |
|---|---|
| Тема | Re: Skipping schema changes in publication |
| Дата | |
| Msg-id | CANhcyEXCKPCAdoqBLAhxt64Nwf+7T52dd8daE3qvhBNTvro13Q@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: Skipping schema changes in publication (Peter Smith <smithpb2250@gmail.com>) |
| Ответы |
Re: Skipping schema changes in publication
Re: Skipping schema changes in publication Re: Skipping schema changes in publication Re: Skipping schema changes in publication |
| Список | pgsql-hackers |
On Fri, 14 Nov 2025 at 12:15, Peter Smith <smithpb2250@gmail.com> wrote: > > Hi Shlok. > > Some review comments for patch v27-0001. > > ====== > doc/src/sgml/ref/alter_publication.sgml > > 1. > + <para> > + The <literal>RESET</literal> clause will reset the publication to > the default > + state. This includes resetting all publication parameters, setting the > + <literal>ALL TABLES</literal> and <literal>ALL SEQUENCES</literal> flags to > + <literal>false</literal>, and removing all associated tables and > schemas from > + the publication. > </para> > > It would be better to give references to the actual > pg_publication.puballtables and .puballsequences flag fields [1] > instead of vaguely calling them the "<literal>ALL TABLES</literal> and > <literal>ALL SEQUENCES</literal> flags". > Fixed > ====== > src/backend/commands/publicationcmds.c > > AlterPublicationReset: > > 2. > + if (pubform->puballtables) > + CacheInvalidateRelcacheAll(); > > Does that also need to check ->puballsequences? > I think we call CacheInvalidateRelcacheAll to invalide the relsync cache for the case of ALTER Publication. For sequences we do not build RelSyncEntry. Also I see there are other similar occurrences (such as RemovePublicationById, AlterPublicationOptions) where we do not invalidate cache if we modify all sequence publications. So, I think we do not require this check for puballsequences. > ====== > src/test/regress/sql/publication.sql > > 3. > If you want to, you can easily combine many of these test cases and > verify them in one go instead of separate ALTER/RESET for every kind > of flag. > > ~~~ > I agree. I have made the changes in the latest patch. > 4. > +-- Verify that 'ALL TABLES' flag is reset > > Missing test to check the 'ALL SEQUENCES' flag gets reset? > Added the test. > ====== > [1] https://www.postgresql.org/docs/devel/catalog-pg-publication.html > 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, Shlok Kyal
Вложения
В списке pgsql-hackers по дате отправления: