Thanks Dinesh. I believe Ashesh is working on the remaining issues, so I'll await a cumulative update from him before I test further.
On Wed, Apr 24, 2013 at 2:39 PM, Dinesh Kumar <dinesh.ku...@enterprisedb.com > wrote: > Hi Ashesh, > > Thank you very much for your guidance on resolving the above issues. As of > now, i have debugged the debugger and fixed some of the issues, as per the > Dave's previous conversation. Please find the below status on this. > > > *- The layout of the parameters dialogue needs work - the grid needs to > size to the dialogue.* > Fixed. > > > *- The checkboxes in the grid are jumbo sized. We have them in the Edit > Grid already - can the code be reused? > * > Fixed. > > *- The popup activity dialog is kinda annoying. Perhaps we could put a > progress indicator in the status bar? Low priority. > * > Not Fixed, since it's a low priority i have scheduled it as my last task. > > *- 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.* > Fixed. > > *- If a function is re-executed in the same session, the return value > isn't cleared from the Results grid.* > Fixed. > > > *- 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. > * > I have been tried a lot to resolve this issue with my trivial knowledge, > but i am not able to figure our out how to fix this. Apologizes for that. > As per my understanding, the re-start process of debugger, closing the > previous session handler and creating new session, may be this is the > reason, dbgController::ResultStack()'s fetch break point operation not able > to get the breakpoints of the previous session handler. > * > * > *- Aborting a direct debug session mid-function caused an indefinite(?) > hang with a busy cursor.* > As per our discussion, you have fixed this for the windows, i haven't > tested the fix for the mac. Sorry :) > * > - 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. > * > Fixed. > > Adding the patch for the above fixes. > > ** This is the extension to your patch. Requesting you to use vim -d > between your patch and this patch to get my patch. :) > > Kindly let me know, if any thing i missed 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 Thu, Apr 11, 2013 at 10:40 PM, Ashesh Vashi < > ashesh.va...@enterprisedb.com> wrote: > >> 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 >>> >> > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
