Re: Support EXCEPT for TABLES IN SCHEMA publications

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