On Wed, Jan 5, 2022 at 11:04 AM Peter Smith <smithpb2...@gmail.com> wrote: > > > 11. src/backend/utils/cache/relcache.c - GetRelationPublicationActions > > Something seemed slightly fishy with the code doing the memcpy, > because IIUC is possible for the GetRelationPublicationInfo function > to return without setting the relation->rd_pubactions. Is it just > missing an Assert or maybe a comment to say such a scenario is not > possible in this case because the is_publishable_relation was already > tested? >
I think it would be good to have an Assert for a valid value of relation->rd_pubactions before doing memcpy. Alternatively, in function, GetRelationPublicationInfo(), we can have an Assert when rd_rfcol_valid is true. I think we can add comments atop GetRelationPublicationInfo about pubactions. > > 13. src/include/utils/rel.h - comment typos > > @@ -164,6 +164,13 @@ typedef struct RelationData > PublicationActions *rd_pubactions; /* publication actions */ > > /* > + * true if the columns referenced in row filters from all the publications > + * the relation is in are part of replica identity, or the publication > + * actions do not include UPDATE and DELETE. > + */ > > Some minor rewording of the comment: > ... > "UPDATE and DELETE" --> "UPDATE or DELETE" > The existing comment seems correct to me. Hou-San can confirm it once as I think this is written by him. -- With Regards, Amit Kapila.