On Mon, Oct 4, 2021 at 4:55 AM vignesh C <vignesh21@gmail.com> wrote:
>
> Attached v36 patch has the changes for the same.
>
I have some comments on the v36-0001 patch:
src/backend/commands/publicationcmds.c
(1)
GetPublicationSchemas()
+ /* Find all publications associated with the schema */
+ pubschsrel = table_open(PublicationNamespaceRelationId, AccessShareLock);
I think the comment is not correct. It should say:
+ /* Find all schemas associated with the publication */
(2)
AlterPublicationSchemas
I think that a comment should be added for the following lines,
something like the comment used in the similar check in
AlterPublicationTables():
+ if (!schemaidlist && stmt->action != DEFELEM_SET)
+ return;
(3)
CheckAlterPublication
Minor comment fix suggested:
BEFORE:
+ * Check if relations and schemas can be in given publication and throws
AFTER:
+ * Check if relations and schemas can be in a given publication and throw
(4)
LockSchemaList()
Suggest re-word of comment, to match imperative comment style used
elsewhere in this code.
BEFORE:
+ * The schemas specified in the schema list are locked in AccessShareLock mode
AFTER:
+ * Lock the schemas specified in the schema list in AccessShareLock mode
src/backend/commands/tablecmds.c
(5)
Code has been added to prevent a table being set (via ALTER TABLE) to
UNLOGGED if it is part of a publication, but I found that I could
still add all tables of a schema having a table that is UNLOGGED:
postgres=# create schema sch;
CREATE SCHEMA
postgres=# create unlogged table sch.test(i int);
CREATE TABLE
postgres=# create publication pub for table sch.test;
ERROR: cannot add relation "test" to publication
DETAIL: Temporary and unlogged relations cannot be replicated.
postgres=# create publication pub for all tables in schema sch;
CREATE PUBLICATION
Regards,
Greg Nancarrow
Fujitsu Australia