Re: pgAdmin Event Trigger Compatibility

Hmm, I can't reproduce it now either. Oh well, thanks for checking. Patch
applied!

On Thu, Sep 5, 2013 at 6:27 PM, Dinesh Kumar
<dinesh(dot)kumar(at)enterprisedb(dot)com>wrote:

> Hi Dave,
>
> Sorry for the big delay on this issue.
>
> I have tried to re-produce the above case in mac but unfortunately i am
> not able to re-produce it. I have followed the below steps to reproduce the
> case.
>
> 1) Got the new pgAdmin's master branch.
>
> 2) Applied the given patch.
>
> 3) Created a new event trigger function under the schema.
>
> 4) Created new event trigger.
>
> 5) Modified the created event trigger function's comment from the schema
> node.
>
> 6) Clicked on "OK".
>
> The above steps are working fine.
>
> Kindly let me know, if i miss anything here.
>
> Thanks in advance.
>
> Dinesh
>
> --
> *Dinesh Kumar*
> Software Engineer
>
> Ph: +918087463317
> Skype ID: dinesh.kumar432
> www.enterprisedb.co <http://www.enterprisedb.com/>m<http://www.enterprisedb.com/>
> *
> Follow us on Twitter*
> @EnterpriseDB
>
> Visit EnterpriseDB for tutorials, webinars, whitepapers<http://www.enterprisedb.com/resources-community> and
> more <http://www.enterprisedb.com/resources-community>
>
>
> On Wed, Jul 31, 2013 at 6:27 PM, Dinesh Kumar <
> dinesh(dot)kumar(at)enterprisedb(dot)com> wrote:
>
>> Hi Dave,
>>
>> Thanks for your inputs.
>>
>> On Wed, Jul 31, 2013 at 6:00 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>
>>> Hi
>>>
>>> On Tue, Jul 30, 2013 at 5:16 PM, Dinesh Kumar
>>> <dinesh(dot)kumar(at)enterprisedb(dot)com> wrote:
>>> > Hi Dave,
>>> >
>>> > On Mon, Jul 29, 2013 at 9:22 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>> >>
>>> >> Hi
>>> >>
>>> >>
>>> >> On Fri, Jul 26, 2013 at 5:58 PM, Dinesh Kumar
>>> >> <dinesh(dot)kumar(at)enterprisedb(dot)com> 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.
>>> >>
>>>
>> I have applied this patch and tested in windows and linux with the above
>> steps. But, it's not get crashing in windows/linux.
>>
>> Apologizes, i haven't tested this in Mac. Let me setup pgAdmin build in
>> mac, and will try to fix this issue as well.
>>
>> Thank you once again.
>>
>> >>
>>> >> 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.
>>>
>>> I tweaked the patch to clarify the comments and some variable names
>>> (see attached), then tested it by changing the comment on an event
>>> trigger function from under the schema. I got the following crash
>>> after clicking OK:
>>>
>>> Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
>>> 0 libwx_base_carbonu-2.8.0.dylib 0x0158cc8a wxListBase::Clear() + 78
>>> 1 libwx_macu_core-2.8.0.dylib 0x011ae948
>>> wxListLineData::~wxListLineData() + 72
>>> 2 libwx_macu_core-2.8.0.dylib 0x011a7105
>>> wxListMainWindow::DoDeleteAllItems() + 527
>>> 3 libwx_macu_core-2.8.0.dylib 0x011ac54c
>>> wxListMainWindow::DeleteEverything() + 120
>>> 4 libwx_macu_core-2.8.0.dylib 0x011ac57b
>>> wxGenericListCtrl::ClearAll() + 23
>>> 5 libwx_macu_core-2.8.0.dylib 0x0115bead wxListCtrl::ClearAll() + 27
>>> 6 pgAdmin3-Debug 0x002ba07c frmMain::ResetLists() + 32
>>> 7 pgAdmin3-Debug 0x002ba643
>>> frmMain::execSelChange(wxTreeItemId, bool) + 35
>>> 8 pgAdmin3-Debug 0x002b7723
>>> frmMain::OnTreeSelChanged(wxTreeEvent&) + 61
>>> 9 libwx_base_carbonu-2.8.0.dylib 0x0154b130
>>> wxAppConsole::HandleEvent(wxEvtHandler*, void
>>> (wxEvtHandler::*)(wxEvent&), wxEvent&) const + 42
>>> 10 libwx_base_carbonu-2.8.0.dylib 0x015cc25b
>>> wxEvtHandler::ProcessEventIfMatches(wxEventTableEntryBase const&,
>>> wxEvtHandler*, wxEvent&) + 125
>>> 11 libwx_base_carbonu-2.8.0.dylib 0x015cd371
>>> wxEventHashTable::HandleEvent(wxEvent&, wxEvtHandler*) + 221
>>> 12 libwx_base_carbonu-2.8.0.dylib 0x015cca6c
>>> wxEvtHandler::ProcessEvent(wxEvent&) + 194
>>> 13 libwx_base_carbonu-2.8.0.dylib 0x015cca83
>>> wxEvtHandler::ProcessEvent(wxEvent&) + 217
>>> 14 libwx_macu_core-2.8.0.dylib 0x0123ceca
>>> wxWindowBase::TryParent(wxEvent&) + 66
>>> 15 libwx_base_carbonu-2.8.0.dylib 0x015cca97
>>> wxEvtHandler::ProcessEvent(wxEvent&) + 237
>>> 16 libwx_base_carbonu-2.8.0.dylib 0x015cca83
>>> wxEvtHandler::ProcessEvent(wxEvent&) + 217
>>> 17 libwx_macu_core-2.8.0.dylib 0x0126456c
>>> wxScrollHelperEvtHandler::ProcessEvent(wxEvent&) + 40
>>> 18 libwx_macu_core-2.8.0.dylib 0x012723b7
>>> wxGenericTreeCtrl::DoSelectItem(wxTreeItemId const&, bool, bool) + 743
>>> 19 libwx_macu_core-2.8.0.dylib 0x0126e4ec
>>> wxGenericTreeCtrl::OnMouse(wxMouseEvent&) + 2866
>>> 20 libwx_base_carbonu-2.8.0.dylib 0x0154b130
>>> wxAppConsole::HandleEvent(wxEvtHandler*, void
>>> (wxEvtHandler::*)(wxEvent&), wxEvent&) const + 42
>>> 21 libwx_base_carbonu-2.8.0.dylib 0x015cc25b
>>> wxEvtHandler::ProcessEventIfMatches(wxEventTableEntryBase const&,
>>> wxEvtHandler*, wxEvent&) + 125
>>> 22 libwx_base_carbonu-2.8.0.dylib 0x015cd371
>>> wxEventHashTable::HandleEvent(wxEvent&, wxEvtHandler*) + 221
>>> 23 libwx_base_carbonu-2.8.0.dylib 0x015cca6c
>>> wxEvtHandler::ProcessEvent(wxEvent&) + 194
>>> 24 libwx_base_carbonu-2.8.0.dylib 0x015cca83
>>> wxEvtHandler::ProcessEvent(wxEvent&) + 217
>>> 25 libwx_macu_core-2.8.0.dylib 0x0126456c
>>> wxScrollHelperEvtHandler::ProcessEvent(wxEvent&) + 40
>>> 26 libwx_macu_core-2.8.0.dylib 0x0118ef16
>>> wxMacTopLevelMouseEventHandler(OpaqueEventHandlerCallRef*,
>>> OpaqueEventRef*, void*) + 1286
>>> 27 libwx_macu_core-2.8.0.dylib 0x0118c0e8
>>> wxMacTopLevelEventHandler(OpaqueEventHandlerCallRef*, OpaqueEventRef*,
>>> void*) + 4344
>>> 28 com.apple.HIToolbox 0x910f39bb
>>> _InvokeEventHandlerUPP(OpaqueEventHandlerCallRef*, OpaqueEventRef*,
>>> void*, long (*)(OpaqueEventHandlerCallRef*, OpaqueEventRef*, void*)) +
>>> 36
>>> 29 com.apple.HIToolbox 0x90f7b394
>>> DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*,
>>> HandlerCallRec*) + 1343
>>> 30 com.apple.HIToolbox 0x90f7a780
>>> SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*,
>>> HandlerCallRec*) + 430
>>> 31 com.apple.HIToolbox 0x90f8e655 SendEventToEventTarget + 88
>>> 32 com.apple.HIToolbox 0x90fae5c7
>>> ToolboxEventDispatcherHandler(OpaqueEventHandlerCallRef*,
>>> OpaqueEventRef*, void*) + 2141
>>> 33 com.apple.HIToolbox 0x90f7b83f
>>> DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*,
>>> HandlerCallRec*) + 2538
>>> 34 com.apple.HIToolbox 0x90f7a780
>>> SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*,
>>> HandlerCallRec*) + 430
>>> 35 com.apple.HIToolbox 0x90f8e655 SendEventToEventTarget + 88
>>> 36 libwx_macu_core-2.8.0.dylib 0x0112e055
>>> wxApp::MacHandleOneEvent(void*) + 41
>>> 37 libwx_macu_core-2.8.0.dylib 0x0112e148 wxApp::MacDoOneEvent() + 144
>>> 38 libwx_macu_core-2.8.0.dylib 0x0114736e wxEventLoop::Dispatch() + 32
>>> 39 libwx_macu_core-2.8.0.dylib 0x011e7e25 wxEventLoopManual::Run() +
>>> 131
>>> 40 libwx_macu_core-2.8.0.dylib 0x011c14cb wxAppBase::MainLoop() + 67
>>> 41 libwx_base_carbonu-2.8.0.dylib 0x01580dae wxEntry(int&, wchar_t**) +
>>> 110
>>> 42 libwx_base_carbonu-2.8.0.dylib 0x01580e62 wxEntry(int&, char**) + 50
>>> 43 pgAdmin3-Debug 0x000a768e main + 30
>>> 44 libdyld.dylib 0x95f55725 start + 1
>>>
>>>
>>> --
>>> Dave Page
>>> Blog: http://pgsnake.blogspot.com
>>> Twitter: @pgsnake
>>>
>>> EnterpriseDB UK: http://www.enterprisedb.com
>>> The Enterprise PostgreSQL Company
>>>
>>
>>
>>
>> Dinesh
>>
>> --
>> *Dinesh Kumar*
>> Software Engineer
>>
>> Ph: +918087463317
>> Skype ID: dinesh.kumar432
>> www.enterprisedb.co <http://www.enterprisedb.com/>m<http://www.enterprisedb.com/>
>> *
>> Follow us on Twitter*
>> @EnterpriseDB
>>
>> Visit EnterpriseDB for tutorials, webinars, whitepapers<http://www.enterprisedb.com/resources-community> and
>> more <http://www.enterprisedb.com/resources-community>
>>
>>
>

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company