Обсуждение: PATCH: Debugger Redesign

Поиск
Список
Период
Сортировка

PATCH: Debugger Redesign

От
Ashesh Vashi
Дата:
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

Вложения

Re: PATCH: Debugger Redesign

От
Dave Page
Дата:
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.vashi@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


Re: PATCH: Debugger Redesign

От
Ashesh Vashi
Дата:

Sure.
I'll work on it...

On 11 Apr 2013 21:00, "Dave Page" <dpage@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.vashi@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

Re: PATCH: Debugger Redesign

От
Dinesh Kumar
Дата:
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.com

Follow us on Twitter

@EnterpriseDB 

Visit EnterpriseDB for tutorials, webinars, whitepapers and more


On Thu, Apr 11, 2013 at 10:40 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

Sure.
I'll work on it...

On 11 Apr 2013 21:00, "Dave Page" <dpage@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.vashi@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

Вложения

Re: PATCH: Debugger Redesign

От
Dinesh Kumar
Дата:
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.

B
est Regards,
Dinesh

Вложения

Re: PATCH: Debugger Redesign

От
Dave Page
Дата:
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.kumar@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
Skype ID: dinesh.kumar432
www.enterprisedb.com

Follow us on Twitter

@EnterpriseDB 

Visit EnterpriseDB for tutorials, webinars, whitepapers and more


On Thu, Apr 11, 2013 at 10:40 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

Sure.
I'll work on it...

On 11 Apr 2013 21:00, "Dave Page" <dpage@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.vashi@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

Re: PATCH: Debugger Redesign

От
Ashesh Vashi
Дата:
On Thu, Apr 25, 2013 at 7:25 PM, Dave Page <dpage@pgadmin.org> wrote:
Thanks Dinesh. I believe Ashesh is working on the remaining issues, so I'll await a cumulative update from him before I test further.
Yeah - I was working on the same.
It took a lot more time than I anticipated. (I found some other bugs too. i.e. fetching frame information on click one of the stacks.)

Here is the updated patch.
On Wed, Apr 24, 2013 at 2:39 PM, Dinesh Kumar <dinesh.kumar@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
Skype ID: dinesh.kumar432
www.enterprisedb.com

Follow us on Twitter

@EnterpriseDB 

Visit EnterpriseDB for tutorials, webinars, whitepapers and more


On Thu, Apr 11, 2013 at 10:40 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

Sure.
I'll work on it...

On 11 Apr 2013 21:00, "Dave Page" <dpage@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.vashi@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



--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:
Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi

Вложения

Re: PATCH: Debugger Redesign

От
Dave Page
Дата:
Thanks Ashesh - committed (with a few minor tweaks, mostly to messages)! Good work :-)

I think we have the following issues to discuss and possibly resolve during beta:

1) I find the progress dialogue annoying, and as discussed previously think we should replace it with a progress indicator on the status bar.

2) I don't see RAISE DEBUG messages in the server message pane. Perhaps we should explicitly set client_min_messages so the user can see their debug messages?

3) The stack pane shows functions as "foo(integer)(param_name=1)@<line>". I think perhaps we should change it to: "foo(integer param_name=1)@<line>"

4) When in-process debugging, if the calling process is terminated we get a connection lost error (which I special-cased in the code, as it displays an unsightly message from the plugin by default). Instead, I think we should just automatically start listening for another calling process.

Thoughts?

Thanks!


On Fri, Apr 26, 2013 at 9:27 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Thu, Apr 25, 2013 at 7:25 PM, Dave Page <dpage@pgadmin.org> wrote:
Thanks Dinesh. I believe Ashesh is working on the remaining issues, so I'll await a cumulative update from him before I test further.
Yeah - I was working on the same.
It took a lot more time than I anticipated. (I found some other bugs too. i.e. fetching frame information on click one of the stacks.)

Here is the updated patch.
On Wed, Apr 24, 2013 at 2:39 PM, Dinesh Kumar <dinesh.kumar@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
Skype ID: dinesh.kumar432
www.enterprisedb.com

Follow us on Twitter

@EnterpriseDB 

Visit EnterpriseDB for tutorials, webinars, whitepapers and more


On Thu, Apr 11, 2013 at 10:40 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

Sure.
I'll work on it...

On 11 Apr 2013 21:00, "Dave Page" <dpage@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.vashi@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



--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:
Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi




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

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

Re: PATCH: Debugger Redesign

От
Ashesh Vashi
Дата:
On Mon, Apr 29, 2013 at 10:30 PM, Dave Page <dpage@pgadmin.org> wrote:
Thanks Ashesh - committed (with a few minor tweaks, mostly to messages)! Good work :-)
Thanks 

I think we have the following issues to discuss and possibly resolve during beta:

1) I find the progress dialogue annoying, and as discussed previously think we should replace it with a progress indicator on the status bar.
Yeah - I am aware of that.
I will work on it once I get some time. 

2) I don't see RAISE DEBUG messages in the server message pane. Perhaps we should explicitly set client_min_messages so the user can see their debug messages?
Yup 

3) The stack pane shows functions as "foo(integer)(param_name=1)@<line>". I think perhaps we should change it to: "foo(integer param_name=1)@<line>"
+1 

4) When in-process debugging, if the calling process is terminated we get a connection lost error (which I special-cased in the code, as it displays an unsightly message from the plugin by default). Instead, I think we should just automatically start listening for another calling process.
The problem is: this error message is coming from backend (debugger plugin).
And hence, it may require us to make changes in the plugin.

And - once again thanks for giving me this opportunity.

Thoughts?

Thanks!


On Fri, Apr 26, 2013 at 9:27 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Thu, Apr 25, 2013 at 7:25 PM, Dave Page <dpage@pgadmin.org> wrote:
Thanks Dinesh. I believe Ashesh is working on the remaining issues, so I'll await a cumulative update from him before I test further.
Yeah - I was working on the same.
It took a lot more time than I anticipated. (I found some other bugs too. i.e. fetching frame information on click one of the stacks.)

Here is the updated patch.
On Wed, Apr 24, 2013 at 2:39 PM, Dinesh Kumar <dinesh.kumar@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
Skype ID: dinesh.kumar432
www.enterprisedb.com

Follow us on Twitter

@EnterpriseDB 

Visit EnterpriseDB for tutorials, webinars, whitepapers and more


On Thu, Apr 11, 2013 at 10:40 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:

Sure.
I'll work on it...

On 11 Apr 2013 21:00, "Dave Page" <dpage@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.vashi@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



--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:
Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi




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

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



--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:
Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi

Re: PATCH: Debugger Redesign

От
Dave Page
Дата:



On Mon, Apr 29, 2013 at 8:10 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Mon, Apr 29, 2013 at 10:30 PM, Dave Page <dpage@pgadmin.org> wrote:
Thanks Ashesh - committed (with a few minor tweaks, mostly to messages)! Good work :-)
Thanks 

I think we have the following issues to discuss and possibly resolve during beta:

1) I find the progress dialogue annoying, and as discussed previously think we should replace it with a progress indicator on the status bar.
Yeah - I am aware of that.
I will work on it once I get some time. 

2) I don't see RAISE DEBUG messages in the server message pane. Perhaps we should explicitly set client_min_messages so the user can see their debug messages?
Yup 

3) The stack pane shows functions as "foo(integer)(param_name=1)@<line>". I think perhaps we should change it to: "foo(integer param_name=1)@<line>"
+1 

OK, please make those tweaks when you work on the progress indicator (or before, if you get a chance).
 

4) When in-process debugging, if the calling process is terminated we get a connection lost error (which I special-cased in the code, as it displays an unsightly message from the plugin by default). Instead, I think we should just automatically start listening for another calling process.
The problem is: this error message is coming from backend (debugger plugin).
And hence, it may require us to make changes in the plugin.

Shouldn't do - don't we just need to reset the breakpoint and wait for it to be hit again? FYI, the old code does seem to do it successfully, though it doesn't do a good job of ensuring the dialogues are re-displayed. 

Thanks!

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

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

Re: PATCH: Debugger Redesign

От
Ashesh Vashi
Дата:
On Tue, Apr 30, 2013 at 12:48 AM, Dave Page <dpage@pgadmin.org> wrote:
On Mon, Apr 29, 2013 at 8:10 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Mon, Apr 29, 2013 at 10:30 PM, Dave Page <dpage@pgadmin.org> wrote:
Thanks Ashesh - committed (with a few minor tweaks, mostly to messages)! Good work :-)
Thanks 

I think we have the following issues to discuss and possibly resolve during beta:

1) I find the progress dialogue annoying, and as discussed previously think we should replace it with a progress indicator on the status bar.
Yeah - I am aware of that.
I will work on it once I get some time. 

2) I don't see RAISE DEBUG messages in the server message pane. Perhaps we should explicitly set client_min_messages so the user can see their debug messages?
Yup 

3) The stack pane shows functions as "foo(integer)(param_name=1)@<line>". I think perhaps we should change it to: "foo(integer param_name=1)@<line>"
+1 

OK, please make those tweaks when you work on the progress indicator (or before, if you get a chance).
 

4) When in-process debugging, if the calling process is terminated we get a connection lost error (which I special-cased in the code, as it displays an unsightly message from the plugin by default). Instead, I think we should just automatically start listening for another calling process.
The problem is: this error message is coming from backend (debugger plugin).
And hence, it may require us to make changes in the plugin.

Shouldn't do - don't we just need to reset the breakpoint and wait for it to be hit again? FYI, the old code does seem to do it successfully, though it doesn't do a good job of ensuring the dialogues are re-displayed.
It was ignore any error in the debugging connection (even if it is a connection error or something else).
I will come up with some other way to figure out the same. (I will also take care of this, when doing the modification for the progress bar. Hope - that's ok with everybody). 

Thanks!

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

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



--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:
Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi

Re: PATCH: Debugger Redesign

От
Ashesh Vashi
Дата:
Hi Dave,

Please find the patch to resolve few of the problems with the debuggers and introduced during redesigning it.

On Tue, Apr 30, 2013 at 12:56 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Tue, Apr 30, 2013 at 12:48 AM, Dave Page <dpage@pgadmin.org> wrote:
On Mon, Apr 29, 2013 at 8:10 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Mon, Apr 29, 2013 at 10:30 PM, Dave Page <dpage@pgadmin.org> wrote:
Thanks Ashesh - committed (with a few minor tweaks, mostly to messages)! Good work :-)
Thanks 

I think we have the following issues to discuss and possibly resolve during beta:

1) I find the progress dialogue annoying, and as discussed previously think we should replace it with a progress indicator on the status bar.
Yeah - I am aware of that.
I will work on it once I get some time. 

2) I don't see RAISE DEBUG messages in the server message pane. Perhaps we should explicitly set client_min_messages so the user can see their debug messages?
Yup 
Done. 

3) The stack pane shows functions as "foo(integer)(param_name=1)@<line>". I think perhaps we should change it to: "foo(integer param_name=1)@<line>"
+1 

OK, please make those tweaks when you work on the progress indicator (or before, if you get a chance).
 

4) When in-process debugging, if the calling process is terminated we get a connection lost error (which I special-cased in the code, as it displays an unsightly message from the plugin by default). Instead, I think we should just automatically start listening for another calling process.
The problem is: this error message is coming from backend (debugger plugin).
And hence, it may require us to make changes in the plugin.

Shouldn't do - don't we just need to reset the breakpoint and wait for it to be hit again? FYI, the old code does seem to do it successfully, though it doesn't do a good job of ensuring the dialogues are re-displayed.
It was ignore any error in the debugging connection (even if it is a connection error or something else).
I will come up with some other way to figure out the same. (I will also take care of this, when doing the modification for the progress bar. Hope - that's ok with everybody).
Done.
Now - we check for the session (backend) on which the target was running, if it does not exist, we wait for another session to invoke the target.

Also - resolved couple of other problem related to the debugger:
- On cancellation of progress dialog, it was not closing the debugger.
- In direct debugging, the target was invoked without taking care about quotes.

This patch also includes couple of other issues introduced by the earlier patch.
1. Showing the messages multiple times in the message window (Query window)
2. Cancellation of query wasn't working on Query window.

Still working on the progress-bar.

Thanks!

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

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



--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:
Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:
Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi

Вложения

Re: PATCH: Debugger Redesign

От
Dave Page
Дата:
Thanks, applied.


On Thu, Jun 20, 2013 at 11:07 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Hi Dave,

Please find the patch to resolve few of the problems with the debuggers and introduced during redesigning it.

On Tue, Apr 30, 2013 at 12:56 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Tue, Apr 30, 2013 at 12:48 AM, Dave Page <dpage@pgadmin.org> wrote:
On Mon, Apr 29, 2013 at 8:10 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Mon, Apr 29, 2013 at 10:30 PM, Dave Page <dpage@pgadmin.org> wrote:
Thanks Ashesh - committed (with a few minor tweaks, mostly to messages)! Good work :-)
Thanks 

I think we have the following issues to discuss and possibly resolve during beta:

1) I find the progress dialogue annoying, and as discussed previously think we should replace it with a progress indicator on the status bar.
Yeah - I am aware of that.
I will work on it once I get some time. 

2) I don't see RAISE DEBUG messages in the server message pane. Perhaps we should explicitly set client_min_messages so the user can see their debug messages?
Yup 
Done. 

3) The stack pane shows functions as "foo(integer)(param_name=1)@<line>". I think perhaps we should change it to: "foo(integer param_name=1)@<line>"
+1 

OK, please make those tweaks when you work on the progress indicator (or before, if you get a chance).
 

4) When in-process debugging, if the calling process is terminated we get a connection lost error (which I special-cased in the code, as it displays an unsightly message from the plugin by default). Instead, I think we should just automatically start listening for another calling process.
The problem is: this error message is coming from backend (debugger plugin).
And hence, it may require us to make changes in the plugin.

Shouldn't do - don't we just need to reset the breakpoint and wait for it to be hit again? FYI, the old code does seem to do it successfully, though it doesn't do a good job of ensuring the dialogues are re-displayed.
It was ignore any error in the debugging connection (even if it is a connection error or something else).
I will come up with some other way to figure out the same. (I will also take care of this, when doing the modification for the progress bar. Hope - that's ok with everybody).
Done.
Now - we check for the session (backend) on which the target was running, if it does not exist, we wait for another session to invoke the target.

Also - resolved couple of other problem related to the debugger:
- On cancellation of progress dialog, it was not closing the debugger.
- In direct debugging, the target was invoked without taking care about quotes.

This patch also includes couple of other issues introduced by the earlier patch.
1. Showing the messages multiple times in the message window (Query window)
2. Cancellation of query wasn't working on Query window.

Still working on the progress-bar.

Thanks!

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

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



--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:
Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:
Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi




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

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

Re: PATCH: Debugger Redesign

От
Ashesh Vashi
Дата:
Here is one more patch.
This patch includes modification related to replacing the progress dialog with a progress bar in the statusbar.

On Thu, Jun 20, 2013 at 7:13 PM, Dave Page <dpage@pgadmin.org> wrote:
Thanks, applied.



On Thu, Jun 20, 2013 at 11:07 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Hi Dave,

Please find the patch to resolve few of the problems with the debuggers and introduced during redesigning it.

On Tue, Apr 30, 2013 at 12:56 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Tue, Apr 30, 2013 at 12:48 AM, Dave Page <dpage@pgadmin.org> wrote:
On Mon, Apr 29, 2013 at 8:10 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
On Mon, Apr 29, 2013 at 10:30 PM, Dave Page <dpage@pgadmin.org> wrote:
Thanks Ashesh - committed (with a few minor tweaks, mostly to messages)! Good work :-)
Thanks 

I think we have the following issues to discuss and possibly resolve during beta:

1) I find the progress dialogue annoying, and as discussed previously think we should replace it with a progress indicator on the status bar.
Yeah - I am aware of that.
I will work on it once I get some time. 

2) I don't see RAISE DEBUG messages in the server message pane. Perhaps we should explicitly set client_min_messages so the user can see their debug messages?
Yup 
Done. 

3) The stack pane shows functions as "foo(integer)(param_name=1)@<line>". I think perhaps we should change it to: "foo(integer param_name=1)@<line>"
+1 

OK, please make those tweaks when you work on the progress indicator (or before, if you get a chance).
 

4) When in-process debugging, if the calling process is terminated we get a connection lost error (which I special-cased in the code, as it displays an unsightly message from the plugin by default). Instead, I think we should just automatically start listening for another calling process.
The problem is: this error message is coming from backend (debugger plugin).
And hence, it may require us to make changes in the plugin.

Shouldn't do - don't we just need to reset the breakpoint and wait for it to be hit again? FYI, the old code does seem to do it successfully, though it doesn't do a good job of ensuring the dialogues are re-displayed.
It was ignore any error in the debugging connection (even if it is a connection error or something else).
I will come up with some other way to figure out the same. (I will also take care of this, when doing the modification for the progress bar. Hope - that's ok with everybody).
Done.
Now - we check for the session (backend) on which the target was running, if it does not exist, we wait for another session to invoke the target.

Also - resolved couple of other problem related to the debugger:
- On cancellation of progress dialog, it was not closing the debugger.
- In direct debugging, the target was invoked without taking care about quotes.

This patch also includes couple of other issues introduced by the earlier patch.
1. Showing the messages multiple times in the message window (Query window)
2. Cancellation of query wasn't working on Query window.

Still working on the progress-bar.

Thanks!

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

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



--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:
Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi

--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:
Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi




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

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



--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:
Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi

Вложения

Re: PATCH: Debugger Redesign

От
Dave Page
Дата:
Hi


On Thu, Jun 20, 2013 at 8:39 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Here is one more patch.
This patch includes modification related to replacing the progress dialog with a progress bar in the statusbar.


Yeah, I think that's much better. I think however, that we should always show the progress bar on the status bar - otherwise, it looks a little weird seeing it flashing on and off, and when it disappears leaving the text inset. What do you think?

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

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

Re: PATCH: Debugger Redesign

От
Ashesh Vashi
Дата:
Like this?
Please find the updated patch.

On Fri, Jun 21, 2013 at 6:39 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi


On Thu, Jun 20, 2013 at 8:39 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Here is one more patch.
This patch includes modification related to replacing the progress dialog with a progress bar in the statusbar.


Yeah, I think that's much better. I think however, that we should always show the progress bar on the status bar - otherwise, it looks a little weird seeing it flashing on and off, and when it disappears leaving the text inset. What do you think?

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

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



--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:
Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi

Вложения

Re: PATCH: Debugger Redesign

От
Dave Page
Дата:
Thanks - I've applied that (I need to cut the beta now, and didn't want to miss this). There are a couple of minor issues - please address them as soon as you can:

- As discussed, we should make the progress bar the second field on the bar, not the first.

- On Windows, the initial sizing is incorrect. Upon a resize of the window, everything redraws correctly. Please see the attached screenshots.

Thanks. 

On Mon, Jun 24, 2013 at 6:01 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Like this?
Please find the updated patch.


On Fri, Jun 21, 2013 at 6:39 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi


On Thu, Jun 20, 2013 at 8:39 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Here is one more patch.
This patch includes modification related to replacing the progress dialog with a progress bar in the statusbar.


Yeah, I think that's much better. I think however, that we should always show the progress bar on the status bar - otherwise, it looks a little weird seeing it flashing on and off, and when it disappears leaving the text inset. What do you think?

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

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



--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:
Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi




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

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Вложения

Re: PATCH: Debugger Redesign

От
Ashesh Vashi
Дата:
Please find the new patch to take care about the position and size of the progress bar in the statusbar.
It also hides the progress-bar from the statusbar, if number of fileds are reduced by the user.

On Mon, Jun 24, 2013 at 8:26 PM, Dave Page <dpage@pgadmin.org> wrote:
Thanks - I've applied that (I need to cut the beta now, and didn't want to miss this). There are a couple of minor issues - please address them as soon as you can:

- As discussed, we should make the progress bar the second field on the bar, not the first.

- On Windows, the initial sizing is incorrect. Upon a resize of the window, everything redraws correctly. Please see the attached screenshots.

Thanks. 

On Mon, Jun 24, 2013 at 6:01 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Like this?
Please find the updated patch.


On Fri, Jun 21, 2013 at 6:39 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi


On Thu, Jun 20, 2013 at 8:39 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Here is one more patch.
This patch includes modification related to replacing the progress dialog with a progress bar in the statusbar.


Yeah, I think that's much better. I think however, that we should always show the progress bar on the status bar - otherwise, it looks a little weird seeing it flashing on and off, and when it disappears leaving the text inset. What do you think?

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

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



--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:
Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi




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

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



--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:
Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi

Вложения

Re: PATCH: Debugger Redesign

От
Dave Page
Дата:
Looks good - committed, thanks!


On Tue, Jun 25, 2013 at 9:17 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Please find the new patch to take care about the position and size of the progress bar in the statusbar.
It also hides the progress-bar from the statusbar, if number of fileds are reduced by the user.


On Mon, Jun 24, 2013 at 8:26 PM, Dave Page <dpage@pgadmin.org> wrote:
Thanks - I've applied that (I need to cut the beta now, and didn't want to miss this). There are a couple of minor issues - please address them as soon as you can:

- As discussed, we should make the progress bar the second field on the bar, not the first.

- On Windows, the initial sizing is incorrect. Upon a resize of the window, everything redraws correctly. Please see the attached screenshots.

Thanks. 

On Mon, Jun 24, 2013 at 6:01 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Like this?
Please find the updated patch.


On Fri, Jun 21, 2013 at 6:39 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi


On Thu, Jun 20, 2013 at 8:39 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Here is one more patch.
This patch includes modification related to replacing the progress dialog with a progress bar in the statusbar.


Yeah, I think that's much better. I think however, that we should always show the progress bar on the status bar - otherwise, it looks a little weird seeing it flashing on and off, and when it disappears leaving the text inset. What do you think?

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

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



--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:
Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi




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

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



--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:
Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi




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

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

Re: PATCH: Debugger Redesign

От
Ashesh Vashi
Дата:

Thanks

On 25 Jun 2013 18:16, "Dave Page" <dpage@pgadmin.org> wrote:
Looks good - committed, thanks!


On Tue, Jun 25, 2013 at 9:17 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Please find the new patch to take care about the position and size of the progress bar in the statusbar.
It also hides the progress-bar from the statusbar, if number of fileds are reduced by the user.


On Mon, Jun 24, 2013 at 8:26 PM, Dave Page <dpage@pgadmin.org> wrote:
Thanks - I've applied that (I need to cut the beta now, and didn't want to miss this). There are a couple of minor issues - please address them as soon as you can:

- As discussed, we should make the progress bar the second field on the bar, not the first.

- On Windows, the initial sizing is incorrect. Upon a resize of the window, everything redraws correctly. Please see the attached screenshots.

Thanks. 

On Mon, Jun 24, 2013 at 6:01 AM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Like this?
Please find the updated patch.


On Fri, Jun 21, 2013 at 6:39 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi


On Thu, Jun 20, 2013 at 8:39 PM, Ashesh Vashi <ashesh.vashi@enterprisedb.com> wrote:
Here is one more patch.
This patch includes modification related to replacing the progress dialog with a progress bar in the statusbar.


Yeah, I think that's much better. I think however, that we should always show the progress bar on the status bar - otherwise, it looks a little weird seeing it flashing on and off, and when it disappears leaving the text inset. What do you think?

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

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



--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:
Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi




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

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



--
--

Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA:
Enterprise PostgreSQL Company

 

http://www.linkedin.com/in/asheshvashi




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

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