Sure. I'll work on it... On 11 Apr 2013 21:00, "Dave Page" <dp...@pgadmin.org> wrote:
> Hi > > First impressions - it already seems more stable than the old code, > and I only played with it for an hour or so on Mac so far. Nice work > :-) > > I did find a few things in my initial testing that need some > attention. I didn't get the impression anything was particularly > serious or would be troublesome to fix though: > > - The layout of the parameters dialogue needs work - the grid needs to > size to the dialogue. > > - The checkboxes in the grid are jumbo sized. We have them in the Edit > Grid already - can the code be reused? > > - The popup activity dialog is kinda annoying. Perhaps we could put a > progress indicator in the status bar? Low priority. > > - When injecting new values into variables, if the value can't be > accepted (wrong datatype etc), then the grid should be reset to the > original value when the error message box is accepted. > > - If a function is re-executed in the same session, the return value > isn't cleared from the Results grid. > > - If a function is re-executed in the same session, the breakpoints > are cleared. This doesn't seem to happen with in-process debugging, > only direct. > > - Aborting a direct debug session mid-function caused an indefinite(?) > hang with a busy cursor. > > - The status messages in the status bar are a little confusing. I > think you need to add "Done." to the end of the string when an action > completes, otherwise it looks like things never complete until you > step or run. > > If you can fix those issues (with the possible exception of the > progress indicator), I'll dive in a bit deeper. I don't want to go too > deep just yet in case you have to change more than I expect. > > Thanks. > > On Wed, Apr 10, 2013 at 12:10 PM, Ashesh Vashi > <ashesh.va...@enterprisedb.com> wrote: > > Hi All, > > > > We always wanted to redesign the plpgsql debugger in pgAdmin III from > > very long time due to very long list of bug > > Reports of the debugger. We were not able crack down variety of cases > across > > the platforms. For many cases - resolving > > On one platform, we ended up introducing other issues on another > platforms. > > This inability of handling variety of cases > > Across platforms made us to redesign the debugger from the scratch. > > > > Let me list down few the limitations/flaws with the current > implementation > > of the debugger: > > - Uses its own mechanism for making connection, querying (async/sync) & > > handing results instead of using the common > > available classes, which are used by all other components in pgAdmin > III. > > i.e. > > dbgConn, dbgResult, dbgPgThread, etc. > > > > Because of this, the improvements in the existing infrastructure were > > never reflected in the debugger. > > i.e. > > The connection with debugger were never be created with the SSL > support. > > Because - dbgConn never supported the SSL. > > This is also results for the many bugs. > > i.e. > > The dbgPgThread has its own mechanism to run queries asynchronously > and > > resulted into many bugs. It generates few > > events, which shouldn't be done after the completion of the debugger > > operations. And, that leads to crash. Using > > TestDestroy() function for JOINABLE threads were unnecessary and never > > required. But - it is using it. > > > > - A hidden windows is taking care of the events generated during the > > debugging the actual window. It could have been > > Good, if all the tasks having intialited and handled from by this > central > > mechanism, instead we do have different > > places for handling both the direct/indirect debugging. And, that > > introduced a lot of issues. And, that made the > > things very confusing even to understand the existing mechanism. > > > > And, many more. > > > > We had to made changes in the existing infrastructure/classes to use it > with > > the debugger. > > * pgConn: > > + Report error quietly (if told) while error found in execution, this > > allows these functions to be used from any > > Thread. Otherwise, it used to throw an error. > > + Set/Reset the PGcancel object before and after the query execution, > > which will be useful for us to use the new > > Mechanism of libpq (of course, not very new) to cancel any query > > execution from in between. > > + Allow new application name, while duplicating the connection > > + Save the version and the features list, while duplicating the > > connection, this will avoid/reduce the duplicate calls > > To the database server. > > * pgQueryThread: > > + Allow to specify the custom notice processor and handler > > + Allow to run multiple queries by creating queue > > + Allow to use the EnterpriseDB's callable statement from the > > pgQueryThread (if available) > > + Proper cancellation mechanism implementation > > + Send a custom event to the caller (instead of standard event) for > > successful execution or on different errors > > (but - not if it is explicitly cancelled by the user.) This allows > us to > > send more information to the developer. > > + Allows the developer to add a batch queries (which should run > > asynchronously) even after starting of the thread. > > + Allow to use the PGparam now, to avoid generating the queries > > dynamically > > > > The new design is based on the MVC (Model-View-Controller) architecture. > > * Controller (dbgController) > > - A central mechanism to handle all the operations. > > * View (frmDebugger - mostly reused) > > - Responsible for presentation and interaction with the end-user > > i.e. > > code viewer, different variable windows, stack window, etc. > > - It also controls other part of view. > > * Model (dbgModel) > > - Handles/Contains all the data > > - It contains the information about the target function/procedure. > > > > For any operation, View and Model will ask the Controller to handle it. > > > > Summary of the changes: > > > > INSERTED| DELETED| FILENAME | > SUMMARY > > > ---------+----------+---------------------------------------------|---------------------------- > > 3| 1| pgadmin/ctl/ctlSQLResult.cpp | - > Call > > cancel thread execution on exit, if already > > | > running > > for nice exit handling > > 196| 18| pgadmin/db/pgConn.cpp | - > > Set/Reset the PGcancel object > > | - > Saving > > settings and new application name, while > > | > cloning > > 31| 24| pgadmin/include/db/pgConn.h | > > 245| 33| pgadmin/include/db/pgQueryThread.h | > > 647| 111| pgadmin/db/pgQueryThread.cpp | - > > Multiple queries execution support (one-by-one) > > 8| 0| pgadmin/include/db/pgSet.h | - > > Introduce a new function GetCommandStatus for the > > | > result > > 0| 1| pgadmin/db/pgSet.cpp | - > > Removed unnecessary header inclusion > > 0| 1563| pgadmin/debugger/ctlCodeWindow.cpp | - > > Omitted/Removed it completely > > 0| 250| pgadmin/include/debugger/ctlCodeWindow.h | > > 9| 9| pgadmin/debugger/ctlMessageWindow.cpp | - > > Standardize the function naming conversion with the > > | other > > components > > 5| 12| pgadmin/include/debugger/ctlMessageWindow.h | > > 28| 27| pgadmin/debugger/ctlResultGrid.cpp | - > Using > > the standard querying mechanism (pgSet) > > | > instead > > of libpq's PGresult directly > > 4| 3| pgadmin/include/debugger/ctlResultGrid.h | > > 4| 7| pgadmin/debugger/ctlStackWindow.cpp | - > > Standardize the function naming conversion with the > > | other > > components > > 2| 2| pgadmin/include/debugger/ctlStackWindow.h | > > 50| 56| pgadmin/debugger/ctlTabWindow.cpp | - > > Standardize the function naming conversion with the > > | other > > components > > 20| 17| pgadmin/include/debugger/ctlTabWindow.h | > > 60| 55| pgadmin/debugger/ctlVarWindow.cpp | - > > Standardize the function naming conversion with the > > | other > > components > > 17| 15| pgadmin/include/debugger/ctlVarWindow.h | > > 819| 0| pgadmin/debugger/dbgController.cpp | - > > Central of the debugger (it takes care of all the > > | > > operations, the frmDebugger (view) and dbgModel > > | > (model) > > interact with it. > > 173| 0| pgadmin/include/debugger/dbgController.h | > > 0| 21| pgadmin/debugger/dbgDbResult.cpp | - > > Omitted/Removed it completely > > 703| 0| pgadmin/debugger/dbgEvents.cpp | - > Added > > new file, which contains all the event > > | > handling > > functions of the dbgController to reduce > > | the > size > > of dbgController and separating them from > > | the > > direct operations > > 63| 0| pgadmin/debugger/dbgModel.cpp | - > > Contains all the information regarding the > > | > target. > > 143| 0| pgadmin/include/debugger/dbgModel.h | > > 0| 544| pgadmin/debugger/dbgPgConn.cpp | - > > Omitted/Removed it completely > > 0| 363| pgadmin/debugger/dbgPgThread.cpp | - > > Omitted/Removed it completely > > 0| 157| pgadmin/debugger/dbgResultset.cpp | - > > Omitted/Removed it completely > > 536| 194| pgadmin/debugger/dbgTargetInfo.cpp | - It > > fetches and saves the target information from > > | the > > database server (it is no more using the > > | > > deprecated pldbg_target_info function). > > 146| 75| pgadmin/include/debugger/dbgTargetInfo.h | > > 44| 101| pgadmin/debugger/debugger.cpp | - > > Debugger menu factory, modified accordingly new > > | > design > > 770| 682| pgadmin/debugger/dlgDirectDbg.cpp | - > > Handles now only taking input from the end-user > > | - It > > also takes care of converting an expression > > | > input to > > a proper value > > | - It > > also allows the end-user to use the default > > | > value of > > an argument through UI. > > | - It > > does not control the debugger execution any > > | more > > 74| 48| pgadmin/include/debugger/dlgDirectDbg.h | > > 734| 165| pgadmin/debugger/frmDebugger.cpp | - > Mostly > > reused > > | - > Works > > as presentation layer and takes input from > > | from > the > > end-user > > | i.e. > > set/reset break-points, changing parameter > > | > > values, showing the target code, showing > > | > > current parameter values, etc. > > 114| 92| pgadmin/include/debugger/frmDebugger.h | > > 3| 5| pgadmin/debugger/module.mk | > > 5| 1| pgadmin/dlg/dlgClasses.cpp | - > Call > > cancel thread execution on exit, if already > > | > running > > for nice exit handling (ExecutionDialog) > > 4| 1| pgadmin/frm/frmEditGrid.cpp | - > Call > > cancel thread execution on abort > > 2| 2| pgadmin/frm/frmQuery.cpp | - > > Modified to use new custom event on query > > | > > execution completion > > 2| 1| pgadmin/include/frm/frmQuery.h | > > 1| 1| pgadmin/include/db/module.mk | > > 79| 0| pgadmin/include/db/pgQueryResultEvent.h | - > > Introduced a new custom event for handling the > > | query > > execution error/success (pgQueryThread) > > 14| 21| pgadmin/include/debugger/dbgBreakPoint.h | - > > Reformatted the break-point according to new design > > 0| 38| pgadmin/include/debugger/dbgConnProp.h | - > > Omitted/Removed it completely > > 57| 10| pgadmin/include/debugger/dbgConst.h | > > 0| 53| pgadmin/include/debugger/dbgDbResult.h | - > > Omitted/Removed it completely > > 0| 99| pgadmin/include/debugger/dbgPgConn.h | - > > Omitted/Removed it completely > > 0| 95| pgadmin/include/debugger/dbgPgThread.h | - > > Omitted/Removed it completely > > 0| 52| pgadmin/include/debugger/dbgResultset.h | - > > Omitted/Removed it completely > > 2| 6| pgadmin/include/debugger/module.mk | > > 3| 6| pgadmin/include/precomp.h | > > 7| 12| pgadmin/pgAdmin3.vcxproj | - > Window > > build script changes > > 10| 28| pgadmin/pgAdmin3.vcxproj.filters | > > 3| 3| pgadmin/ii/dlgDirectDbg.xrc | - > Input > > Arguments dialog change (changed the > > | > > dimensions.) > > 44| 44| pgadmin/ii/xrcDialogs.cpp | > > > > > > -- > > > > Thanks & Regards, > > > > Ashesh Vashi > > EnterpriseDB INDIA: Enterprise PostgreSQL Company > > > > > > http://www.linkedin.com/in/asheshvashi > > > > > > > > -- > > Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) > > To make changes to your subscription: > > http://www.postgresql.org/mailpref/pgadmin-hackers > > > > > > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >
