Re: Support EXCEPT for ALL SEQUENCES publications

On Tue, 30 Jun 2026 at 13:17, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
>
> Hi,
>
> On Tue, Jun 30, 2026 at 12:14 PM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
> >
> > On Mon, 29 Jun 2026 at 06:02, Peter Smith <smithpb2250@gmail.com> wrote:
> > >
> > > Hi Shlok.
> > >
> > > I only had one general comment about v14. IMO, it would be better to
> > > try to make the regression test object names more meaningful where
> > > possible (though sometimes it won't be). The xxx1, xxx2, and xxx3
> > > become harder to read as more gets added.
> > >
> > > e.g. `regress_pub_seq3` -- What does 'pub' mean here? This is an
> > > unlogged sequence you want to exclude.
> > > e.g. `regress_pub_forallsequences4` -- Doesn't mean much.
> > > e.g. `tab_seq` -- This is meant to be a plain table; Not a table
> > > pretending to be a seq.
> > >
> > > Below is an example of some modifications, but there are many more.
> > > Consider checking all the names to see if they can be improved
> > > (sometimes you may be stuck having to accommodate existing names).
> > > (same comment applies to both patches).
> > >
> > > ~~~
> > >
> > > BEFORE
> > > +-- fail - unlogged sequence is specified in EXCEPT sequence list
> > > +CREATE UNLOGGED SEQUENCE regress_pub_seq3;
> > > +CREATE PUBLICATION regress_pub_forallsequences4 FOR ALL SEQUENCES
> > > EXCEPT (SEQUENCE regress_pub_seq3);
> > > +
> > > +-- fail - temporary sequence is specified in EXCEPT sequence list
> > > +CREATE TEMPORARY SEQUENCE regress_pub_seq4;
> > > +CREATE PUBLICATION regress_pub_forallsequences4 FOR ALL SEQUENCES
> > > EXCEPT (SEQUENCE regress_pub_seq4);
> > > +
> > > +-- fail - sequence object is specified in EXCEPT table list
> > > +CREATE PUBLICATION regress_pub_forallsequences4 FOR ALL TABLES EXCEPT
> > > (TABLE regress_pub_seq0);
> > > +
> > > +-- fail - table object is specified in EXCEPT sequence list
> > > +CREATE TABLE tab_seq(a int);
> > > +CREATE PUBLICATION regress_pub_forallsequences4 FOR ALL SEQUENCES
> > > EXCEPT (SEQUENCE tab_seq);
> > > +
> > >
> > >
> > > SUGGESTION
> > > +-- fail - unlogged sequence is specified in EXCEPT sequence list
> > > +CREATE UNLOGGED SEQUENCE regress_seq_unlogged;
> > > +CREATE PUBLICATION regress_pub_should_fail FOR ALL SEQUENCES EXCEPT
> > > (SEQUENCE regress_seq_unlogged);
> > > +
> > > +-- fail - temporary sequence is specified in EXCEPT sequence list
> > > +CREATE TEMPORARY SEQUENCE regress_seq_temp;
> > > +CREATE PUBLICATION regress_pub_should_fail FOR ALL SEQUENCES EXCEPT
> > > (SEQUENCE regress_seq_temp);
> > > +
> > > +-- fail - sequence object is specified in EXCEPT table list
> > > +CREATE PUBLICATION regress_pub_should_fail FOR ALL TABLES EXCEPT
> > > (TABLE regress_seq1);
> > > +
> > > +-- fail - table object is specified in EXCEPT sequence list
> > > +CREATE TABLE tab1(a int);
> > > +CREATE PUBLICATION regress_pub_should_fail FOR ALL SEQUENCES EXCEPT
> > > (SEQUENCE tab1);
> > > +
> > >
> > I agree with your suggestions, I have made the changes.
> > I have also made changes for other objects as well in publication.sql
> > and 037_except.pl files.
> >
> > I also agree with the suggestions by Shveta in [1] and made the
> > required changes.
> >
> > Please find the updated v15 patch attached.
> >
>
> + /* TODO : Add a version check for PG 20 */
> + if (pset.sversion >= 190000)
> + {
> + /* Get sequences in the EXCEPT clause for this publication */
> + printfPQExpBuffer(&buf, "/* %s */\n",
> +   _("Get sequences excluded by this publication"));
> + printfPQExpBuffer(&buf,
> +   "SELECT n.nspname || '.' || c.relname\n"
> +   "FROM pg_catalog.pg_class c\n"
> +   "     JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace\n"
> +   "     JOIN pg_catalog.pg_publication_rel pr ON c.oid = pr.prrelid\n"
> +   "WHERE pr.prpubid = '%s' AND pr.prexcept AND c.relkind = 'S'\n"
> +   "ORDER BY 1", pubid);
> + if (!addFooterToPublicationDesc(&buf, _("Except sequences:"),
> + true, &cont))
> + goto error_return;
> + }
> + }
>
> I've few questions related to above change:
>
> 1) Should the second printfPQExpBuffer() be appendPQExpBuffer()? As
> written, it overwrites the query comment added just above, so psql -E
> will not show "Get sequences excluded by this publication".
>
I agree, the second printfPQExpBuffer() should be appendPQExpBuffer().
Made the change.

> 2) The above query may work on PG 19, but since the feature is going
> to be part of PG20, why do you want this query to be part of PG19?
>
I added the checks for PG19, as the major version on master was still
pointing to PG19.
But now I noticed that the major version is now bumped to PG20, I have
modified the checks

> --
>
> Do we also need to include a tap test for "ALTER PUBLICATION ... SET
> ALL SEQUENCES EXCEPT (...)" followed by "ALTER SUBSCRIPTION ...
> REFRESH SEQUENCES", and then sequence replication behavior is
> verified?
>
Yes, I agree

I have made the changes and attached the updated v16 patches. I have
also addressed Peter's comment in [1].

[1]: https://www.postgresql.org/message-id/CAHut+Pu_OQubF686DUK0J2fYkL_0ks27HNPOZ6PqSF_qmjDLFA@mail.gmail.com

Thanks,
Shlok Kyal