Some review comments for v9-0001.

======
src/backend/catalog/pg_publication.c

GetExcludedPublicationTables:

1.
- Assert(GetPublication(pubid)->alltables);
+ Assert(GetPublication(pubid)->alltables ||
+    GetPublication(pubid)->allsequences);

Better to assign to a variable first, instead of calling GetPublication() 2x.

~~~

GetAllTablesPublications:

2.
- * For a FOR ALL TABLES publication, the returned list excludes
tables mentioned
- * in the EXCEPT clause.
+ * For a FOR ALL TABLES publication, the returned list excludes tables
+ * mentioned in the EXCEPT clause. For a FOR ALL SEQUENCES publication,
+ * it excludes sequences mentioned in the EXCEPT clause.

(2 times)

/mentioned/specified/ or
/mentioned/named/

~~~

3.
+ /* EXCEPT filtering applies to tables and sequences */
+ exceptlist = GetExcludedPublicationRelations(pubid, pubviaroot ?
+ PUBLICATION_PART_ROOT :
+ PUBLICATION_PART_LEAF);

I don't think the comment is needed anymore. It was relevant before,
when there was a condition, but now there is no condition.

======
src/backend/parser/gram.y

preprocess_pubobj_list:

4.
- /* relation name or pubtable must be set for this type of object */
- if (!pubobj->name && !pubobj->pubtable)
+ /* relation name or pubrelation must be set for this type of object */
+ if (!pubobj->name && !pubobj->pubrelation)
...
- /* convert it to PublicationTable */
- PublicationTable *pubtable = makeNode(PublicationTable);
+ /* convert it to PublicationRelation */
+ PublicationRelation *pubrelation = makeNode(PublicationRelation);

Since you are changing these comments, you can uppercase them
in-passing for consistency.
/relation name/Relation name/
/convert it/Convert it/

======
Kind Regards,
Peter Smith.
Fujitsu Australia


Reply via email to