0002 looks like a good improvement to me. The existing routine is messy, and apparently it's so just to save one LockSharedObject plus cache lookup; IMO it's not worth it. Patched code looks simpler. If there are cases where having the combined behavior is useful, it's not clear what they are. (If I understand correctly, the reason is that a sync worker could try to insert-or-update the row after some other process deleted it [because of removing the table from subscription?] ... but that seems to work out *simpler* with the new code. So what's up?)
-- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
