Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#510 addSQLExecutionListener is broken

Snapshot
closed-fixed
Rob Manning
Core (461)
5
2015-01-08
2007-04-16
tmuerell
No

Hello SquirrelSQL team,

i want to write a plugin that rewrites the executed SQL to replace variables with values. Therefore I want to use the addSQLExecutionListener() method of ISQLPanelAPI. But this is in current CVS HEAD broken, because the listener is added to the panel's listeners list and the actual execution of the SQL is done in the ISQLResultExecuter.

If you fix this, I will provide a plugin that does variable expansion when the SQL is executed.

Greetings,

Thorsten

Discussion

  • Rob Manning
    Rob Manning
    2007-04-16

    Logged In: YES
    user_id=1287991
    Originator: NO

    Thorsten,

    What database dialect are you using as a model for implementing variable substitution?
    For Oracle, SQL-Plus expects a

    DEFINE <key> = <value>

    and then &key to reference the variable. Is it different for other databases that support variable substitution?

    Rob

     
  • tmuerell
    tmuerell
    2007-04-16

    Logged In: YES
    user_id=1671155
    Originator: YES

    Hi Rob,

    i'm actually thinking about client side variable replacing.

    For example, TOAD allows queries like:

    SELECT *
    FROM customer
    WHERE firstname = :firstname
    ;

    When you execute this statement, an input dialog asks for the value of ":firstname" and already has as default the last value you gave for ":firstname" if you execute the statement multiple times.

    This functionality should be independent of the used database.

    Thorsten

     
  • Rob Manning
    Rob Manning
    2007-04-17

    Logged In: YES
    user_id=1287991
    Originator: NO

    I'm not the author of this code, so bear with me - it's a bit complex. It looks like SQLPanel.addExecutor needs to do the following:

    1. ISQLResultExecuter needs addExecutionListener (ISQLExecutionListener)
    (ExplainPlanExecuter and SQLResultExecuterPanel need to then implement this - SQLResultExecuterPanel already does)

    2. SQLPanel.addExecutor needs the following code:

    EventListener[] execListeners =
    _listeners.getListeners(ISQLExecutionListener.class);
    for (int i = 0; i < execListeners.length; i++) {
    EventListener listener = execListeners[i];
    exec.addExecutionListener(listener);
    }

    As a result, any executer that is added will have all of the registered listeners added to it. To pickup executors that were added prior to adding the sql execution listener:

    SQLPanel.addSQLExecutionListener(ISQLExecutionListener lis) {

    ...

    for \(ISQLResultExecuter exec: \_executors\) \{
    

    exec.addExecutionListener(lis);
    }
    }

    For this you would also need to declare _executors in SQLPanel thusly:

    private final List&lt;ISQLResultExecuter&gt; \_executors = new ArrayList&lt;ISQLResultExecuter&gt;\(\);
    

    It would be nice for Gerd or Colin to look at this and make sure I'm not leading you down the wrong path. In any case, it appears that you are correct in that the ISQLExecutionListener being added isn't being used, so this would at least fix that issue.

    Rob

     
  • Rob Manning
    Rob Manning
    2007-04-17

    Logged In: YES
    user_id=1287991
    Originator: NO

    Thorsten,

    Colin had a look at my proposed changes to make the API that you are attempting to use work. He said they looked ok, given the state that the code is in right now. Gerd may be able to provide more guidance as he did alot of work on the result tabs back in 2005. So stay tuned for any clarifications. However, I say you can probably forge ahead if the changes I've proposed work for you - at least if it makes the API work - which doesn't work today work - the code is better for it. You can attach a zip file with patches to this bug report if that is convenient for you.

    Rob

     
  • Rob Manning
    Rob Manning
    2007-04-17

    • assigned_to: colbell --> manningr
     
  • tmuerell
    tmuerell
    2007-04-17

    Logged In: YES
    user_id=1671155
    Originator: YES

    Hello Rob,

    yes, it's ok for me. I'll fix the code to make the listeners work and attach the patches to this report.

    Greetings,

    Thorsten

     
  • tmuerell
    tmuerell
    2007-04-18

    Logged In: YES
    user_id=1671155
    Originator: YES

    File Added: sqlexecutionlistener_app.patch.txt

     
  • tmuerell
    tmuerell
    2007-04-18

    Logged In: YES
    user_id=1671155
    Originator: YES

    File Added: sqlexecutionlistener_oracle.patch.txt

     
  • tmuerell
    tmuerell
    2007-04-18

    Logged In: YES
    user_id=1671155
    Originator: YES

    Hi,

    I've uploaded my patches to solve this issue, but I also saw that something was already done.

    Thank you,

    Thorsten

     
  • Rob Manning
    Rob Manning
    2007-04-20

    Logged In: YES
    user_id=1287991
    Originator: NO

    Thorsten,

    Yes, Gerd checked in code to fix the problem. Could you give that code a try to see if it fixes the issue you were having? It's less code and from someone who is more familiar with that code than I am at at the moment. Let me know if that is workable. If so, I looked at the patch files you submitted and it looks like these won't be necessary if Gerd's fix works for you.

    Rob

     
  • tmuerell
    tmuerell
    2007-04-20

    Logged In: YES
    user_id=1671155
    Originator: YES

    Hello Rob,

    yes, these changes solve the issue for me. It is not really clear if multiple executors can be added or readded, so it's ok for me to go on with the Gerd's modifications.

    Greetings,

    Thorsten

     
  • tmuerell
    tmuerell
    2007-04-20

    • status: open --> closed
     
  • Rob Manning
    Rob Manning
    2007-04-20

    Logged In: YES
    user_id=1287991
    Originator: NO

    Thorsten,

    Thanks for following up on this - I appreciate your flexibility. When you are ready to publish the code for the new plugin, let me know.

    Rob

     
  • Rob Manning
    Rob Manning
    2007-04-20

    • status: closed --> closed-fixed