Re: pgAdmin Event Trigger Compatibility

From: Dinesh Kumar <dinesh(dot)kumar(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: pgAdmin Event Trigger Compatibility
Date: 2013-09-05 17:27:30
Message-ID: CAKWsr7i92c5t=u1FwD6HK-tMC3Y9ooS_P+Bw3=3PwGCnTtsapQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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>
>
>

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2013-09-06 12:28:16 pgAdmin website commit: Cleanup of styling and additional cleanup.
Previous Message Shish 2013-09-05 13:57:02 Re: Some website tweaks