On Tuesday, August 3, 2021 11:08 PM vignesh C <vignes...@gmail.com> wrote:
>
> Thanks for reporting this, this is fixed in the v18 patch attached.
Thanks for fixing it.
Few suggestions for V18:
1.
+# Clean up the tables on both publisher and subscriber as we don't need them
+$node_publisher->safe_psql('postgres', "DROP SCHEMA sch1 cascade");
+$node_publisher->safe_psql('postgres', "DROP SCHEMA sch2 cascade");
+$node_publisher->safe_psql('postgres', "DROP SCHEMA sch3 cascade");
+$node_subscriber->safe_psql('postgres', "DROP SCHEMA sch1 cascade");
+$node_subscriber->safe_psql('postgres', "DROP SCHEMA sch2 cascade");
+$node_subscriber->safe_psql('postgres', "DROP SCHEMA sch3 cascade");
Should we change the comment to "Clean up the schemas ... ", instead of
'tables'?
2.
+$result = $node_subscriber->safe_psql('postgres',
+ "SELECT count(*) FROM sch1.tab3");
Spaces are used here(and some other places), but in most places we use a TAB, so
I think it's better to change it to a TAB.
3.
List *tables; /* List of tables to add/drop */
bool for_all_tables; /* Special publication for all tables
in db */
DefElemAction tableAction; /* What action to perform with the
tables */
+ List *schemas; /* Optional list of schemas */
} AlterPublicationStmt;
Should we change the comment here to 'List of schemas to add/drop', then it can
be consistent with the comment for 'tables'.
I also noticed that 'tableAction' variable is used when we add/drop/set schemas,
so maybe the variable name is not suitable any more.
Besides, the following comment is above these codes. Should we add some comments
for schema?
/* parameters used for ALTER PUBLICATION ... ADD/DROP TABLE */
And it says 'add/drop', do we need to add 'set'? (it's not related to this
patch, so I think I can add it in another thread[1] if needed, which is related
to comment improvement)
4.
I saw the existing tests about permissions in publication.sql, should we add
tests for schema publication? Like this:
diff --git a/src/test/regress/sql/publication.sql
b/src/test/regress/sql/publication.sql
index 33dbdf7bed..c19337631e 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -160,16 +160,19 @@ GRANT CREATE ON DATABASE regression TO
regress_publication_user2;
SET ROLE regress_publication_user2;
SET client_min_messages = 'ERROR';
CREATE PUBLICATION testpub2; -- ok
+CREATE PUBLICATION testpub3; -- ok
RESET client_min_messages;
ALTER PUBLICATION testpub2 ADD TABLE testpub_tbl1; -- fail
+ALTER PUBLICATION testpub3 ADD SCHEMA pub_test; -- fail
SET ROLE regress_publication_user;
GRANT regress_publication_user TO regress_publication_user2;
SET ROLE regress_publication_user2;
ALTER PUBLICATION testpub2 ADD TABLE testpub_tbl1; -- ok
+ALTER PUBLICATION testpub3 ADD SCHEMA pub_test; -- ok
-DROP PUBLICATION testpub2;
+DROP PUBLICATION testpub2, testpub3;
SET ROLE regress_publication_user;
REVOKE CREATE ON DATABASE regression FROM regress_publication_user2;
[1]
https://www.postgresql.org/message-id/flat/OS0PR01MB6113480F937572BF1216DD61FBEF9%40OS0PR01MB6113.jpnprd01.prod.outlook.com
Regards
Tang
