Re: long-standing data loss bug in initial sync of logical replication

Hi Kuroda-san,

Thanks for reviewing the patch.
>
> 1.
> I feel the name of SnapBuildDistributeNewCatalogSnapshot() should be updated because it
> distributes two objects: catalog snapshot and invalidation messages. Do you have good one
> in your mind? I considered "SnapBuildDistributeNewCatalogSnapshotAndInValidations" or
> "SnapBuildDistributeItems" but seems not good :-(.

I have renamed the function to 'SnapBuildDistributeSnapshotAndInval'. Thoughts?

> 2.
> Hmm, still, it is overengineering for me to add a new type of invalidation message
> only for the publication. According to the ExecRenameStmt() we can implement an
> arbitrary rename function like RenameConstraint() and RenameDatabase().
> Regaring the ALTER PUBLICATION OWNER TO, I feel adding CacheInvalidateRelcacheAll()
> and InvalidatePublicationRels() is enough.

I agree with you.

>
> I attached a PoC which implements above. It could pass tests on my env. Could you
> please see it tell me how you think?

I have tested the POC and it is working as expected. The changes look
fine to me. I have created a patch for the same.
Currently, we are passing 'PUBLICATION_PART_ALL' as an argument to
function 'GetPublicationRelations' and
'GetAllSchemaPublicationRelations'. Need to check if we can use
'PUBLICATION_PART_ROOT' or 'PUBLICATION_PART_LEAF' depending on the
'publish_via_partition_root' option. Will test and address this in the
next version of the patch. For now, I have added a TODO.

Thanks and Regards,
Shlok Kyal