Re: Added schema level support for publication.

On Fri, Aug 6, 2021 at 6:32 PM vignesh C <vignesh21(at)gmail(dot)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