Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.0 M2
    • Fix Version/s: 1.0 M3
    • Component/s: JDO
    • Labels:
      None
    • Environment:
      Castor v1.0M2
      Windows XP
    • Number of attachments :
      4

      Description

      We have found an issue trying to log some SQL statements created by castor.

      "updates", "selects", "deletes" and "inserts" sqls are correctly logged with the "PreparedStatementProxy" class.

      However "inserts" where a key generator (with the returning option) is used is not logged.

      That is because, in such environment, a CallableStatement is created, not a PreparedStatement.

      The ConnectionProxy do not wrap the CallableStatement with a "CallableStatementProxy" as it does with the prepared statement.

      Therefore, CallableStatements do not show their SQLs properly.

      We have modified the castor code in order to introduce a new "CallableStatementProxy" class that do the same that the "PreparedStatementProxy" one but right for callable statements (It is almost a perfect copy of "PreparedStatementProxy").

      We have modified the "ConnectionProxy" class, as well, in order to use this "CallableStatementProxy".

      The version of the "CallableStatementProxy" provided in the patch is a copy of the PreparedStetementProxy that uses CallableStatements instead of PreparedStatements. It registers, as well, all the parameter values entered in order to write them correctly in the "toString()" method, which is exactly the same as the one from "PreparedStatementProxy".

      Nevertheless, all the additional methods of the CallableStatement interface just delegate to the internal "callableStatement" member variable without storing the parameters as those from the "preparedStatementProxy". That could be, maybe, improved for future releases (perhaps we have been very lazy about this, sorry).

      Thanks,

      Raúl.

      P.D.: I hope the patch is well done. It is the first time I do one. Please, tell me if there is something wrong.

      1. log_callableStatemens02.patch
        51 kB
        Raúl Sanz de Acedo Pérez
      2. patch.C1316.20060210.txt
        47 kB
        Werner Guttmann
      3. patch.C1316.20060210-002.txt
        49 kB
        Werner Guttmann
      4. patch.C1316.20060210-003.txt
        115 kB
        Werner Guttmann

        Issue Links

          Activity

          Hide
          Raúl Sanz de Acedo Pérez added a comment -

          the first patch "log_callableStatemens.patch" includes some useless things, sorry.

          Show
          Raúl Sanz de Acedo Pérez added a comment - the first patch "log_callableStatemens.patch" includes some useless things, sorry.
          Hide
          Werner Guttmann added a comment -

          Thanks, Raul, for supplying me with this patch. In the future, it would be even better if you supplied me with a unified patch (as opposed to a simple patch/diff). But as you are attaching two new files, this is not an issue with this issue.

          Show
          Werner Guttmann added a comment - Thanks, Raul, for supplying me with this patch. In the future, it would be even better if you supplied me with a unified patch (as opposed to a simple patch/diff). But as you are attaching two new files, this is not an issue with this issue.
          Hide
          Raúl Sanz de Acedo Pérez added a comment -

          Sorry. But the first patch is not right. Only the second one "log_callableStatemens02.patch". It includes the changes of the "ConnectionProxy" and the new "CallableStatementProxy" class.

          Show
          Raúl Sanz de Acedo Pérez added a comment - Sorry. But the first patch is not right. Only the second one "log_callableStatemens02.patch". It includes the changes of the "ConnectionProxy" and the new "CallableStatementProxy" class.
          Hide
          Werner Guttmann added a comment -

          Original patch re-attached as 'unified patch'.

          Show
          Werner Guttmann added a comment - Original patch re-attached as 'unified patch'.
          Hide
          Werner Guttmann added a comment -

          Final patch, including CHANGELOG entries.

          Show
          Werner Guttmann added a comment - Final patch, including CHANGELOG entries.
          Hide
          Ralf Joachim added a comment -

          In additon I'd like to see the useProxy property being removed. Instead we should check:

          LogFactory.getLog(ConnectionProxy.class).isDebugEnabled()

          If above call returns true the connection should be wrapped by a ConnectionProxy, otherwise the connection will not be wrapped. This allows the debugging to be configured only by the underlaying logging system at one place and not being splitted around in various places.

          Show
          Ralf Joachim added a comment - In additon I'd like to see the useProxy property being removed. Instead we should check: LogFactory.getLog(ConnectionProxy.class).isDebugEnabled() If above call returns true the connection should be wrapped by a ConnectionProxy, otherwise the connection will not be wrapped. This allows the debugging to be configured only by the underlaying logging system at one place and not being splitted around in various places.
          Hide
          Ralf Joachim added a comment -

          You may take a look at how this is done with cache now. Having said that a explanation is contained in release notes of 1.0M2.

          Show
          Ralf Joachim added a comment - You may take a look at how this is done with cache now. Having said that a explanation is contained in release notes of 1.0M2.
          Hide
          Werner Guttmann added a comment -

          Final patch (but release notes), with all the *Proxy classes moved to org.castor.jdo.drivers package. And we agreed to address the "useProxies" property and its use as part of CASTOR-1318.

          Show
          Werner Guttmann added a comment - Final patch (but release notes), with all the *Proxy classes moved to org.castor.jdo.drivers package. And we agreed to address the "useProxies" property and its use as part of CASTOR-1318 .
          Hide
          Raúl Sanz de Acedo Pérez added a comment -

          OK, I see. I think it is a very good idea to unify log configuration. Thank you very much for your patience.

          Show
          Raúl Sanz de Acedo Pérez added a comment - OK, I see. I think it is a very good idea to unify log configuration. Thank you very much for your patience.

            People

            • Assignee:
              Werner Guttmann
              Reporter:
              Raúl Sanz de Acedo Pérez
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 25 minutes
                25m