The code in get_rel_sync_entry() changes current memory context to CacheMemoryContext, then does a bunch of memory-leaking things. This is not good, because that memory context has to be very carefully managed to avoid permanent memory leaks. I suppose you added that because you need something -- probably entry->att_map -- to survive memory context resets, but if so then you need to change to CacheMemoryContext only when that memory is allocated, not other chunks of memory. I suspect you can fix this by moving the MemoryContextSwitchTo() to just before calling get_table_columnset; then all the leaky thinkgs are done in whatever the original memory context is, which is fine.
(However, you also need to make sure that ->att_map is carefully freed at the right time. It looks like this already happens in rel_sync_cache_relation_cb, but is rel_sync_cache_publication_cb correct? And in get_rel_sync_entry() itself, what if the entry already has att_map -- should it be freed prior to allocating another one?) By the way, I notice that your patch doesn't add documentation changes, which are of course necessary. /me is left wondering about PGOutputData->publication_names memory handling ... -- Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/
