On Fri, Aug 6, 2021 at 6:32 PM vignesh C <vignesh21@gmail.com> wrote:
>
> Thanks for the comments, the attached v19 patch has the fixes for the comments.
>
Some more review comments, this time for the v19 patch:
(1) In patch v19-0002, there's still a couple of instances where it
says "publication type" instead of "publication kind".
(2) src/backend/catalog/pg_publication.c
"This should only be used for normal publications."
What exactly is meant by that - what type is considered normal? Maybe
that comment should be more specific.
(3) src/backend/catalog/pg_publication.c
GetSchemaPublications
Function header says "Gets list of publication oids for publications
marked as FOR SCHEMA."
Shouldn't it say something like: "Gets the list of FOR SCHEMA
publication oids associated with a specified schema oid." or something
like that?
(since the function accepts a schemaid parameter)
(4) src/backend/commands/publicationcmds.c
In AlterPublicationSchemas(), I notice that the two error cases
"cannot be added to or dropped ..." don't check stmt->action for
DEFELEM_ADD/DEFELEM_DROP.
Is that OK? (i.e. should these cases error out if stmt->action is not
DEFELEM_ADD/DEFELEM_DROP?)
Also, I assume that the else part (not DEFELEM_ADD/DEFELEM_DROP) is
the "Set" case? Maybe a comment should be added to the top of the else
part.
(5) src/backend/commands/publicationcmds.c
Typo (same in 2 places): "automaically" -> "automatically"
+ * will be released automaically at the end of create publication
See functions:
(i) CreatePublication
(ii) AlterPublicationSchemas
(6) src/backend/commands/publicationcmds.c
LockSchemaList
Function header says "are locked in ShareUpdateExclusiveLock mode" but
then code calls LockDatabaseObject using "AccessShareLock".
Regards,
Greg Nancarrow
Fujitsu Australia
