Hi Dave, On Mon, Jul 29, 2013 at 9:22 PM, Dave Page <[email protected]> wrote:
> Hi > > > On Fri, Jul 26, 2013 at 5:58 PM, Dinesh Kumar < > [email protected]> wrote: > >> Hi Dave, >> >> Thanks for committing the patch. >> >> I have found one memory leak issue in the previous implementation and >> would like to submit this new patch on top of the >> Pg_Event_Trigger_V5.patch. Please find the below attached patch and >> valgrind output and let me know your inputs and suggestions. >> > > OK. Some comments: > > + if(newData->GetMetaType() == PGM_SCHEMA && !newData->IsCollection()) > + { > + doNeedEvtTrgRefresh = true; > + } > + > + // Event trigger's backend functions are at schema level. > + // Hence, we can consider the Event Triggers are partially belongs to > at schema level. > + // So, if any schema got refreshed, we are refreshing the event trigger > collection as like schema's object. > + // It's a special case, which effects the schema operations on the > event triggers as well. > + // > + if (doNeedEvtTrgRefresh) > + { > + doNeedEvtTrgRefresh = false; > + > > Refresh(browser->GetObject(browser->GetNextSibling(browser->GetItemParent(browser->GetSelection())))); > + } > > * Why both with the > doNeedEvtTrgRefresh flag here? As it's not used anywhere else, why not > just put the Refresh() call into the first conditional? > > Yes, True. There is no need of Flag doNeedEvtTrgRefresh. > * I assume the Refresh call is there to find the "Event Triggers" node > and refresh it? If so, there's no guarantee that the next sibling will > actually be the event triggers node - in the future, we may add a new node > type that sits in that position, or the user may have enabled or disabled > some node types, including Event Triggers. > > Ah. Thanks Dave for your suggestions. I have followed another approach to fix this issue. Kindly check this latest patch and share you inputs and suggestions. This patch also fixes the memory leak and schema refresh activities. * The same comment as above applies to browser->GetPrevSibling(). > > Thanks Dave. Best Regards, Dinesh Thanks. > > > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >
Event_Trigger_Memoryleak_V2.patch
Description: Binary data
-- Sent via pgadmin-hackers mailing list ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgadmin-hackers
