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
