MIME-Version: 1.0
References: 
 <CABdArM5sw4Q1ZU8HGdo4BSc1A_+8xtUNq17j6wcir=yMUy19Cg@mail.gmail.com>
 <CAHut+PvnH8QHa035Skoh1e9jm_H08DO9fQ=F-NAMsEpYf0RZ2Q@mail.gmail.com>
 <CAJpy0uDu0LcNXcZCP0cR_LHqo+sau33KwPFHemmGVYf_JTxRBQ@mail.gmail.com>
 <CAA4eK1KbCWBmEXH-rhQjKgNwq=onZp8vRR-QkRhPpbKwL-kQdw@mail.gmail.com>
 <CAHut+Pvj4=GWoJEd4EBdp4pi6KxXQ46ioW=PV+=UktiXr2gCvg@mail.gmail.com>
 <CABdArM75F0A+DGP8AOt-_b_XREX40rvFid1jRjnr_+S5b51t8Q@mail.gmail.com>
 <CAJpy0uDTshb243L5yEYWB3uO-JrwSoRqQDNovh03K2GZuuR3Pg@mail.gmail.com>
 <CAJpy0uDy97ULmJUwPacAzc5u2seuPK6RXgCS1rnsW2MfR4eeSw@mail.gmail.com>
 <CABdArM6oXXXSAxxXFktTTfBf4kyxJCvdNtTbUZtSwJ=CepN+Xw@mail.gmail.com>
 <CAJpy0uBqM+fq7+g1ZRATuY16H10MFP9i25wfFCYCE5MGu+PE0Q@mail.gmail.com>
 <CABdArM4uKaS1coCQj6rAwMmHqU_cCJyEWNic-PFF1_ZjDDM82Q@mail.gmail.com>
 <CAHut+Pu5VNakf5JAhKM7T-P_q37eN1Qgv5nvZUe+8RAAT41y4g@mail.gmail.com>
 <CABdArM6WTm2gP4pcjVdHT1Nx6zdLKTq7nLPUkzZOhprc95a6Rw@mail.gmail.com>
 <CAHut+Ptthc1X-UA8-6zG-iFeCDuoNd+oJRBZ1eCnJ9RNOXjfBQ@mail.gmail.com>
 <CABdArM79m7-CTf6KGGGU2QBydFtuonGgfxRSqk-vhwTsH8z1ow@mail.gmail.com>
 <CAHut+PvBjw8JJOksjJsCN+U4Lda0vWAQTYaYy7ucuMMr8stj0w@mail.gmail.com>
 <CABdArM7b_-ZggpCVGDkpRJ1XCja7ovvY9i4KKGh11jHu4xLYwg@mail.gmail.com>
In-Reply-To: 
 <CABdArM7b_-ZggpCVGDkpRJ1XCja7ovvY9i4KKGh11jHu4xLYwg@mail.gmail.com>
From: Peter Smith <smithpb2250@gmail.com>
Date: Fri, 29 May 2026 18:24:16 +1000
Message-ID: 
 <CAHut+PuhL7Xj8UAK0yBmbbDsCC9xvRVmreCC_yxr+bMfc-dt5g@mail.gmail.com>
Subject: Re: Support EXCEPT for TABLES IN SCHEMA publications
To: Nisha Moond <nisha.moond412@gmail.com>
Cc: shveta malik <shveta.malik@gmail.com>,
 Amit Kapila <amit.kapila16@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CAHut%2BPuhL7Xj8UAK0yBmbbDsCC9xvRVmreCC_yxr%2BbMfc-dt5g%40mail.gmail.com>
Precedence: bulk

Hi Nisha.

Some review comments for patch v7-0002.

======
src/backend/commands/publicationcmds.c

1.
+static void AlterPublicationSchemas(AlterPublicationStmt *stmt,
+ HeapTuple tup, List *schemaidlist,
+ List *except_rel_names);
+static void AlterPublicationSchemaExceptTables(AlterPublicationStmt *stmt,
+    HeapTuple tup,
+    List *except_rel_names,
+    List *schemaidlist);

Maybe the same doubts about the `except_rel_names` parameter/variable
are continued into this patch. See patch 0001 where I first queried
this.

======
src/bin/pg_dump/t/002_pg_dump.pl

2.
+ 'CREATE PUBLICATION pub12' => {
+ create_order => 50,
+ create_sql =>
+   'CREATE PUBLICATION pub12 FOR TABLES IN SCHEMA dump_test EXCEPT
(TABLE test_table, dump_test.test_second_table);',
+ regexp => qr/^
+ \QCREATE PUBLICATION pub12 WITH (publish = 'insert, update, delete,
truncate');\E
+ /xm,
+ like => { %full_runs, section_post_data => 1, },
+ },
+
+ 'ALTER PUBLICATION pub12 ADD TABLES IN SCHEMA dump_test EXCEPT
(TABLE test_table, dump_test.test_second_table)'
+   => {
+ regexp => qr/^
+ \QALTER PUBLICATION pub12 ADD TABLES IN SCHEMA dump_test EXCEPT
(TABLE ONLY test_table, TABLE ONLY test_second_table);\E
+ /xm,
+ like => { %full_runs, section_post_data => 1, },
+   },

I found those hard to read at first. How about just changing the test
title of the ALTER parts

BEFORE
+ 'ALTER PUBLICATION pub12 ADD TABLES IN SCHEMA dump_test EXCEPT
(TABLE test_table, dump_test.test_second_table)'
SUGGESTION
+ 'CREATE PUBLICATION pub12 test continues ...'

(2 places like this)

======
src/test/regress/expected/publication.out

3.
+-- DROP TABLES IN SCHEMA also removes associated EXCEPT entries
+ALTER PUBLICATION testpub_alter_except DROP TABLES IN SCHEMA pub_test;
+\dRp+ testpub_alter_except
+                                                       Publication
testpub_alter_except
+          Owner           | All tables | All sequences | Inserts |
Updates | Deletes | Truncates | Generated columns | Via root |
Description
+--------------------------+------------+---------------+---------+---------+---------+-----------+-------------------+----------+-------------
+ regress_publication_user | f          | f             | t       | t
     | t       | t         | none              | f        |
+Except tables:
+    "pub_test.testpub_tbl_s1"
+

Isn't this showing a BUG, because after the DROP the "Except tables"
are still listed.

~~~

4.
+-- ADD: unqualified name is implicitly qualified with the schema, not public
+ALTER PUBLICATION testpub_alter_except ADD TABLES IN SCHEMA pub_test
EXCEPT (TABLE testpub_tbl_s2);
+\dRp+ testpub_alter_except
+                                                       Publication
testpub_alter_except
+          Owner           | All tables | All sequences | Inserts |
Updates | Deletes | Truncates | Generated columns | Via root |
Description
+--------------------------+------------+---------------+---------+---------+---------+-----------+-------------------+----------+-------------
+ regress_publication_user | f          | f             | t       | t
     | t       | t         | none              | f        |
+Tables from schemas:
+    "pub_test"
+Except tables:
+    "pub_test.testpub_tbl_s1"
+    "pub_test.testpub_tbl_s2"
+

Isn't this showing the same BUG as the previous test, because "s1" is
still in the "Except tables" list, even though it was not part of the
EXCEPT.

~~~

5.
+-- ADD: multiple excepted tables using unqualified names
+ALTER PUBLICATION testpub_alter_except DROP TABLES IN SCHEMA pub_test;
+ALTER PUBLICATION testpub_alter_except ADD TABLES IN SCHEMA pub_test
EXCEPT (TABLE testpub_tbl_s1, testpub_tbl_s2);
+ERROR:  relation "pub_test.testpub_tbl_s1" cannot be added because it
is excluded from publication "testpub_alter_except"

Isn't this showing the same BUG again, because "s1" was already in the
EXCEPT list when it should not be.

======
Kind Regards,
Peter Smith.
Fujitsu Australia