On Tue, Nov 11, 2025 at 9:22 PM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
>
> On Fri, 7 Nov 2025 at 09:34, Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > Hi Shlok.
> >
> > This is a general comment about the content of these patches.
> >
> > IIUC, the v25* patches currently are currently arranged like this:
> >
> > 0001
> > - New command ALTER PUBLICATION pubname RESET;
> > 0002
> > - Add new command: ALTER PUBLICATION pub_name ADD ALL TABLES;
> > - Enhance existing CREATE and the new ALTER syntax for EXCEPT tables
> > 0003
> > - Enhance existing CREATE and ALTER syntax for EXCEPT col_list
> >
> > ~~~
> >
> > IMO it is a bug that the ALTER PUBLICATION pub_name ADD/SET ALL TABLES
> > command does not already exist as a supported command. And, that is
> > independent of anything else you are implementing here like RESET or
> > EXCEPT.
> >
> > Therefore, I think that one should be 1st in your patchset; The EXCEPT
> > stuff then just becomes enhancements to existing syntax, which would
> > give a cleaner separation of logic.
> >
> > So, I am suggesting there should be 4 patches instead of 3. e.g.
> >
> > SUGGESTION
> > 0001 - New command: ALTER PUBLICATION pub_name ADD/SET ALL TABLES;
> > 0002 - New command: ALTER PUBLICATION pubname RESET;
> > 0003 - Enhance existing CREATE/ALTER syntax for EXCEPT tables
> > 0004 - Enhance existing CREATE/ALTER syntax for EXCEPT col_list
> >
> I read the previous conversation in the thread. And got an
> understanding that RESET was introduced so that we can have a way to
> remove 'EXCEPT TABLE' from a publication and after RESET we can use
> 'ADD ALL TABLES [EXCEPT]' to alter the list of EXCEPT TABLE. So I
> prefer to keep 'ALTER PUBLICATION .. RESET' as the first patch.
> I think since 'ADD ALL TABLES' serves our current purpose. We can add
> the syntax 'SET ALL TABLES' once 'ADD ALL TABLES' is in committed or
> in committable shape.
>
Sure, you can defer the ALTER PUBLICATION ... SET ALL TABLES.
However, I still think that 'ALTER PUBLICATION ... ADD ALL TABLES' is
a self-contained new command that deserves to have its own *separate*
patch and tests and docs, etc.
IMO, patch 0002 is doing too much at once. It would be tidier (and
smaller and easier to review, etc) if you split 0002 to implement the
new 'ALTER PUBLICATION ... ADD ALL TABLES' separately, before
expanding on that to implement the EXCEPT part: 'ALTER PUBLICATION ...
ADD ALL TABLES [EXCEPT ...]'.
======
Kind Regards,
Peter Smith.
Fujitsu Australia.
