From: Manu <manuelreyesbravo@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: vignesh C <vignesh21@gmail.com>, shveta malik <shveta.malik@gmail.com>,
 pgsql-hackers@lists.postgresql.org
Subject: Re: Distinguish publication exclusions in object addresses
Date: Tue, 22 Sep 2026 20:32:27 -0300
Message-ID: <179011994715.485930.9380133175707344860@gmail.com>
In-Reply-To: 
 <CAA4eK1JCYqWPDVfrqD01Ug6vow2S7BgXXVTkrc25N8s91NTudw@mail.gmail.com>
References: 
 <CAA4eK1JCYqWPDVfrqD01Ug6vow2S7BgXXVTkrc25N8s91NTudw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============3568873159008537116=="
Archived-At: 
 <https://www.postgresql.org/message-id/179011994715.485930.9380133175707344860%40gmail.com>
Precedence: bulk

--===============3568873159008537116==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

Hi Amit,

> Thanks for sharing the testcase patch but I don't find compelling to
> add it along with current patch due to: (a) it is not necessary to use
> temp table to test this patch, (b) I am not sure we want to test all
> negative cases especially with some special handling though I am open
> to consider it separately as a test to increase code-coverage.

Fair enough on both counts, and (a) was right: the temporary table was
incidental to that patch. Here it is as the separate coverage patch you
were open to, and with a reason to exist that does not depend on the
quoting fix.

check_publication_add_relation() can report

    cannot specify relation "%s" in the publication EXCEPT clause

from six different ereport() sites: a partition with an incomplete
detach, an individual partition, an unsupported relkind, a system
table, a conflict log table, and a temporary or unlogged relation.

Of those six, the regression tests currently reach one. Grepping the
expected output for that message finds a single DETAIL:

    DETAIL:  This operation is not supported for individual partitions.

The attached patch covers three more, all in publication.sql next to
the existing non-EXCEPT cases for the same relations:

    view          -> not supported for views.
    system table  -> not supported for system tables.
    temp table    -> not supported for temporary tables.

(the DETAIL of each, verbatim: "This operation is ...")

The first two are plain statements. The temporary one prints the
message with the schema number redacted, because pg_temp_N depends on
the backend; stats_ext.sql already does this. I ran the same block from
three concurrent sessions, which got pg_temp_1, pg_temp_2 and pg_temp_3,
and all three printed the identical line.

make check passes, 239/239.

I left the two paths I could not reach cheaply: the incomplete-detach
case needs an interrupted concurrent detach, and the conflict log table
case needs that namespace to exist. Happy to drop any of the three if
you would rather keep publication.sql shorter - the system table and
view cases are the ones that cost nothing.

Regards,
Manu

--===============3568873159008537116==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
 filename="0001-Add-test-coverage-for-the-publication-EXCEPT-clause-.patch"
MIME-Version: 1.0

From 39bf631df574fdd7e5120541e7c3f8013227d1ef Mon Sep 17 00:00:00 2001
From: Manu <manuelreyesbravo@gmail.com>
Date: Tue, 22 Sep 2026 20:31:45 -0300
Subject: [PATCH] Add test coverage for the publication EXCEPT clause error
 paths

check_publication_add_relation() has six ereport() paths that can report
"cannot specify relation \"%s\" in the publication EXCEPT clause", but the
regression tests only reach one of them, the one for individual partitions.

Cover three more: a view, a system table and a temporary table.  The
temporary schema number depends on the backend, so that case prints the
message with the number redacted, the way stats_ext.sql does.
---
 src/test/regress/expected/publication.out | 21 +++++++++++++++++++++
 src/test/regress/sql/publication.sql      | 18 ++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/src/test/regress/expected/publication.out b/src/test/regress/exp=
ected/publication.out
index b55da39fde5..eae1c5ab335 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -1568,11 +1568,32 @@ DROP TABLE testpub_tbl4;
 CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_view;
 ERROR:  cannot add relation "testpub_view" to publication
 DETAIL:  This operation is not supported for views.
+-- fail - view in the EXCEPT clause
+CREATE PUBLICATION testpub_exceptview FOR ALL TABLES EXCEPT (TABLE testpub_v=
iew);
+ERROR:  cannot specify relation "public.testpub_view" in the publication EXC=
EPT clause
+DETAIL:  This operation is not supported for views.
+-- fail - system table in the EXCEPT clause
+CREATE PUBLICATION testpub_exceptsystbl FOR ALL TABLES EXCEPT (TABLE pg_clas=
s);
+ERROR:  cannot specify relation "pg_catalog.pg_class" in the publication EXC=
EPT clause
+DETAIL:  This operation is not supported for system tables.
 CREATE TEMPORARY TABLE testpub_temptbl(a int);
 -- fail - temporary table
 CREATE PUBLICATION testpub_fortemptbl FOR TABLE testpub_temptbl;
 ERROR:  cannot add relation "testpub_temptbl" to publication
 DETAIL:  This operation is not supported for temporary tables.
+-- fail - temporary table in the EXCEPT clause.  The temporary schema number
+-- depends on the backend, so the message is printed with it redacted.
+DO $$
+DECLARE
+    detail text;
+BEGIN
+    CREATE PUBLICATION testpub_excepttemptbl FOR ALL TABLES EXCEPT (TABLE te=
stpub_temptbl);
+EXCEPTION WHEN invalid_parameter_value THEN
+    GET STACKED DIAGNOSTICS detail =3D PG_EXCEPTION_DETAIL;
+    RAISE NOTICE '% (%)',
+        regexp_replace(SQLERRM, 'pg_temp_[0-9]+', 'pg_temp_REDACTED'), detai=
l;
+END $$;
+NOTICE:  cannot specify relation "pg_temp_REDACTED.testpub_temptbl" in the p=
ublication EXCEPT clause (This operation is not supported for temporary table=
s.)
 DROP TABLE testpub_temptbl;
 CREATE UNLOGGED TABLE testpub_unloggedtbl(a int);
 -- fail - unlogged table
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publ=
ication.sql
index e3dbb2bc57c..dfd1aa83676 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -995,9 +995,27 @@ DROP TABLE testpub_tbl4;
 -- fail - view
 CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_view;
=20
+-- fail - view in the EXCEPT clause
+CREATE PUBLICATION testpub_exceptview FOR ALL TABLES EXCEPT (TABLE testpub_v=
iew);
+
+-- fail - system table in the EXCEPT clause
+CREATE PUBLICATION testpub_exceptsystbl FOR ALL TABLES EXCEPT (TABLE pg_clas=
s);
+
 CREATE TEMPORARY TABLE testpub_temptbl(a int);
 -- fail - temporary table
 CREATE PUBLICATION testpub_fortemptbl FOR TABLE testpub_temptbl;
+-- fail - temporary table in the EXCEPT clause.  The temporary schema number
+-- depends on the backend, so the message is printed with it redacted.
+DO $$
+DECLARE
+    detail text;
+BEGIN
+    CREATE PUBLICATION testpub_excepttemptbl FOR ALL TABLES EXCEPT (TABLE te=
stpub_temptbl);
+EXCEPTION WHEN invalid_parameter_value THEN
+    GET STACKED DIAGNOSTICS detail =3D PG_EXCEPTION_DETAIL;
+    RAISE NOTICE '% (%)',
+        regexp_replace(SQLERRM, 'pg_temp_[0-9]+', 'pg_temp_REDACTED'), detai=
l;
+END $$;
 DROP TABLE testpub_temptbl;
=20
 CREATE UNLOGGED TABLE testpub_unloggedtbl(a int);
--=20
2.55.0


--===============3568873159008537116==--