Re: Support EXCEPT for ALL SEQUENCES publications - Mailing list pgsql-hackers

On Mon, 28 Sept 2026 at 11:11, Peter Smith <smithpb2250@gmail.com> wrote:
>
> Hi Shlok.
>
> I took a look at v26*. The last review I did was v24, so I refer to
> that one several times in this post.
>
> //////////
> PATCH v26-0001
> //////////
>
> ======
> src/sgml/catalogs.sgml
>
> 1.
> The changes that were previously here in v24-0001 to say "tables or
> sequences" instead of just "tables" are now missing.
>
> Why? I suspect it is an accidental omission.
>
> ======
> 2.
> Docs fail to build.
>
> When I try to build the docs, I get the following error:
> postgres.sgml:6606: element link: validity error : IDREF attribute
> linkend references an unknown ID
> "sql-createpublication-params-for-except-table"
>
> ~
>
> AFAICT, that's the same as the catalogs.sgml problem above, because in
> v24-0001 that reference name was changed in catalogs.sgml. But now
> (since catalogs.sgml was omitted (accidentally?) from the patch it's
> using the old reference name, which is now gone.
>
> ======
> src/backend/catalog/pg_publication.c
>
> check_publication_add_relation:
>
> 3.
>   * error if not.
>   */
>  static void
> -check_publication_add_relation(PublicationRelInfo *pri)
> +check_publication_add_relation(PublicationRelInfo *pri, char pubrelkind)
>
> Previous patch v24 described the new `pubrelkind` parameter, but now
> that description is missing. Why was it removed?
>
I have fixed the above comments. These changes were missed during the rebase.
Added them back.

> ======
> src/test/regress/expected/publication.out:
>
> 4.
> +-- fail - EXCEPT (SEQUENCE) clause specifies a temporary sequence
> +CREATE TEMPORARY SEQUENCE regress_seq_temp;
> +CREATE PUBLICATION regress_pub_should_fail FOR ALL SEQUENCES EXCEPT
> (SEQUENCE regress_seq_temp);
> +ERROR:  cannot specify "pg_temp.regress_seq_temp" in the publication
> EXCEPT (SEQUENCE) clause
> +DETAIL:  This operation is not supported for temporary sequences.
>
> The test for temporary sequences used to exist in v24-0001 but it
> seems to have been removed.
>
> Even if you think some extra tests are excessive, IMO it's probably
> better to keep them for now and remove them much later just prior to
> the patch being pushed.
>
Commit 926627b changed how relname is constructed, so temporary
schemas are now reported using names, such as pg_temp_2. Since the
numeric suffix depends on the backend, using a literal expected value
would make the regression test unstable.
I added a test that redacts the backend-specific temporary-schema
suffix, making the expected output stable, and included it in the
patch.
Another possible approach would be to use get_namespace_name_or_temp()
in check_publication_add_relation() when constructing relname, which
would report the temporary schema as pg_temp.

> ~~~
>
> 5.
> +-- fail - EXCEPT (TABLE) clause specifies a sequence object
> +CREATE PUBLICATION regress_pub_should_fail FOR ALL TABLES EXCEPT
> (TABLE regress_seq0);
> +ERROR:  cannot specify relation "public.regress_seq0" in the
> publication EXCEPT clause
> +DETAIL:  This operation is not supported for sequences.
> +-- fail - EXCEPT (SEQUENCE) clause specifies a table object
> +CREATE PUBLICATION regress_pub_should_fail FOR ALL SEQUENCES EXCEPT
> (SEQUENCE regress_tab1);
> +ERROR:  cannot specify sequence "public.regress_tab1" in the
> publication EXCEPT clause
> +DETAIL:  This operation is not supported for tables.
>
> Those errors are misleading, and the ERROR/DETAIL are contradictory, because:
> regress_seq0 is NOT a relation.
> regress_tab1 is NOT a sequence.
>
> Previously (in v24-0001), those error messages were worded in such a
> way (below) that this was not a problem:
> ERROR:  cannot specify "public.regress_seq0" in the publication EXCEPT
> (TABLE) clause
> ERROR:  cannot specify "public.regress_tab1" in the publication EXCEPT
> (SEQUENCE) clause
>
Fixed

> //////////
> PATCH v26-0002
> //////////
>
> ======
> src/test/regress/expected/publication.out:
>
> 1.
>  -- fail - EXCEPT (SEQUENCE) clause specifies a temporary sequence
>  CREATE TEMPORARY SEQUENCE regress_seq_temp;
>  CREATE PUBLICATION regress_pub_should_fail FOR ALL SEQUENCES EXCEPT
> (SEQUENCE regress_seq_temp);
>  ERROR:  cannot specify "pg_temp.regress_seq_temp" in the publication
> EXCEPT (SEQUENCE) clause
>  DETAIL:  This operation is not supported for temporary sequences.
> +ALTER PUBLICATION regress_pub_forallsequences_except SET ALL
> SEQUENCES EXCEPT (SEQUENCE regress_seq_temp);
> +ERROR:  cannot specify "pg_temp.regress_seq_temp" in the publication
> EXCEPT (SEQUENCE) clause
> +DETAIL:  This operation is not supported for temporary sequences.
>
> The above test used to exist in v24-0002, but has been removed in
> v26-0002. IMO testing the exclusion of temporary sequences should
> remain for now (same as review comment #4 for patch 0001).
>
> ~~~
>
Same as (4) above.

> 2.
>  -- fail - EXCEPT (TABLE) clause specifies a sequence object
>  CREATE PUBLICATION regress_pub_should_fail FOR ALL TABLES EXCEPT
> (TABLE regress_seq0);
>  ERROR:  cannot specify relation "public.regress_seq0" in the
> publication EXCEPT clause
>  DETAIL:  This operation is not supported for sequences.
> +ALTER PUBLICATION regress_pub_forallsequences_except SET ALL TABLES
> EXCEPT (TABLE regress_seq0);
> +ERROR:  cannot specify relation "public.regress_seq0" in the
> publication EXCEPT clause
> +DETAIL:  This operation is not supported for sequences.
>  -- fail - EXCEPT (SEQUENCE) clause specifies a table object
>  CREATE PUBLICATION regress_pub_should_fail FOR ALL SEQUENCES EXCEPT
> (SEQUENCE regress_tab1);
>  ERROR:  cannot specify sequence "public.regress_tab1" in the
> publication EXCEPT clause
>  DETAIL:  This operation is not supported for tables.
> +ALTER PUBLICATION regress_pub_forallsequences_except SET ALL
> SEQUENCES EXCEPT (SEQUENCE regress_tab1);
> +ERROR:  cannot specify sequence "public.regress_tab1" in the
> publication EXCEPT clause
> +DETAIL:  This operation is not supported for tables.
>
> Same review comment as #6 above. These messages are confusing, but in
> v24 they were not.
Fixed

Thanks,
Shlok Kyal