Re: Column Filtering in Logical Replication - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: Column Filtering in Logical Replication
Date
Msg-id 202109061916.xxrml7wrmuoe@alvherre.pgsql
Whole thread Raw
In response to Re: Column Filtering in Logical Replication  (Rahila Syed <rahilasyed90@gmail.com>)
List pgsql-hackers
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/



pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: Column Filtering in Logical Replication
Next
From: Tom Lane
Date:
Subject: Re: BUG #15293: Stored Procedure Triggered by Logical Replication is Unable to use Notification Events