<< Back to previous view

[JDBC-132] create-table-ddl fails with wrong specs type Created: 31/May/16  Updated: 31/May/16  Resolved: 31/May/16

Status: Resolved
Project: java.jdbc
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Sanel Zukan Assignee: Sean Corfield
Resolution: Declined Votes: 0
Labels: bug


 Description   

Using:

(db-do-commands db
   (create-table-ddl "bla" {}))

fails with StackOverflow and incomprehensible error. The same applies for empty vector (create-table-ddl "bla" []). Error looks like:


...
DEPRECATED: unrolled column specs / key/value arguments to create-table-ddl
DEPRECATED: unrolled column specs / key/value arguments to create-table-ddl
DEPRECATED: unrolled column specs / key/value arguments to create-table-ddl
DEPRECATED: unrolled column specs / key/value arguments to create-table-ddl
DEPRECATED: unrolled column specs / key/value arguments to create-table-ddl
DEPRECATED: unrolled column specs / key/value arguments to create-table-ddl
DEPRECATED: unrolled column specs / key/value arguments to create-table-ddl
StackOverflowError clojure.lang.RT.nth (RT.java:897)
com.pav.api.db.db=> java.lang.StackOverflowError: null
at clojure.lang.RT.nth (RT.java:897)
clojure.tools.nrepl.middleware.session$session_out$fn__36289.doInvoke (session.clj:33)
clojure.lang.RestFn.invoke (RestFn.java:460)
clojure.tools.nrepl.middleware.session.proxy$java.io.Writer$ff19274a.write (:-1)
java.io.PrintWriter.write (PrintWriter.java:456)
java.io.PrintWriter.write (PrintWriter.java:473)
clojure.core$fn__6086.invokeStatic (core_print.clj:199)
clojure.core/fn (core_print.clj:191)
clojure.lang.MultiFn.invoke (MultiFn.java:233)
...

create-table-ddl should check own argument types and their content, throwing sane error if fails.



 Comments   
Comment by Sean Corfield [ 31/May/16 12:52 PM ]

If you're getting that message, then you're using java.jdbc 0.5.x which was an interim version to help migrate to the new API: DEPRECATED: unrolled column specs / key/value arguments to create-table-ddl.

In 0.6.1, those calls "work" (but produce questionable SQL):

user=> (require '[clojure.java.jdbc :as jdbc])
nil
user=> (jdbc/create-table-ddl "bla" [])
"CREATE TABLE bla ()"
user=> (jdbc/create-table-ddl "bla" {})
"CREATE TABLE bla ()"
user=>

Both of those calls have produced a NullPointerException in pretty much all versions prior to 0.6.0 (I just tried 0.5.5, 0.4.2). 0.5.8 was just supposed to give an additional warning for legal calls that would become illegal in 0.6.0.

Comment by Sean Corfield [ 31/May/16 12:54 PM ]

The calls were never legal and produced a NullPointerException prior to the migration releases (that print the DEPRECATED messages). Whilst those migration releases are less than optimal in their handling of illegal calls, that isn't a regression.

In 0.6.1, the calls are accepted (but probably shouldn't be).





[JDBC-131] db-transaction* doesn't handle .setAutoCommit throwing exception Created: 31/May/16  Updated: 31/May/16

Status: Open
Project: java.jdbc
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Sam Roberton Assignee: Sean Corfield
Resolution: Unresolved Votes: 0
Labels: None


 Description   

db-transaction* executes the function it is given within a (try ... (finally ...)), and within finally it calls (.setAutoCommit con auto-commit) as well as (possibly) .setTransactionIsolation and .setReadOnly, in order to restore the connection's original values for those attributes.

Those three methods are documented as being allowed to throw SQLException – including if the connection is closed, which it may now be precisely because of an exception thrown by the function in the (try ... (finally ...)). jTDS, at least, does throw an exception out of (.setAutoCommit ...) when the original exception has caused the connection to be closed. When this happens, the original exception is swallowed in favour of the one thrown by the (.setAutoCommit ...).

The calls within the finally should probably themselves be wrapped within a (try ... (finally ...)), and discarded in favour of allowing the original exception to propagate if the (.setAutoCommit ...) (or other setter invocation) fails.



 Comments   
Comment by Sean Corfield [ 31/May/16 1:05 AM ]

Thanks Sam. Do you happen to have repro cases of any of these, which could be added to the test cases?

JDBC seems to be full of unpleasant edge cases

Comment by Sam Roberton [ 31/May/16 2:04 AM ]

I did just manage to reproduce it with the following (using with-db-transaction rather than directly calling db-transaction*, just for convenience):

(clojure.java.jdbc/with-db-transaction [tx {:classname "net.sourceforge.jtds.jdbc.Driver"
                                            :subprotocol "jtds:sqlserver"
                                            :subname "//<DB-HOST>/<DB-NAME>;socketTimeout=10"
                                            :user "<DB-USER>"
                                            :password "<DB-PASSWORD>"}]
  (clojure.java.jdbc/query tx ["waitfor delay '00:00:15'"]))

The exception I get in CIDER is:

Unhandled java.sql.SQLException
   Invalid state, the Connection object is closed.

      ConnectionJDBC2.java: 1713  net.sourceforge.jtds.jdbc.ConnectionJDBC2/checkOpen
      ConnectionJDBC2.java: 2223  net.sourceforge.jtds.jdbc.ConnectionJDBC2/setAutoCommit
                  jdbc.clj:  612  clojure.java.jdbc/db-transaction*
                  jdbc.clj:  618  clojure.java.jdbc/db-transaction*
                  jdbc.clj:  587  clojure.java.jdbc/db-transaction*
                      REPL:  118  user/eval76047
             Compiler.java: 6782  clojure.lang.Compiler/eval
             Compiler.java: 6745  clojure.lang.Compiler/eval
                  core.clj: 3081  clojure.core/eval
                  main.clj:  240  clojure.main/repl/read-eval-print/fn
                  main.clj:  240  clojure.main/repl/read-eval-print
                  main.clj:  258  clojure.main/repl/fn
                  main.clj:  258  clojure.main/repl
               RestFn.java: 1523  clojure.lang.RestFn/invoke
    interruptible_eval.clj:   87  clojure.tools.nrepl.middleware.interruptible-eval/evaluate/fn
                  AFn.java:  152  clojure.lang.AFn/applyToHelper
                  AFn.java:  144  clojure.lang.AFn/applyTo
                  core.clj:  630  clojure.core/apply
                  core.clj: 1868  clojure.core/with-bindings*
               RestFn.java:  425  clojure.lang.RestFn/invoke
    interruptible_eval.clj:   85  clojure.tools.nrepl.middleware.interruptible-eval/evaluate
    interruptible_eval.clj:  222  clojure.tools.nrepl.middleware.interruptible-eval/interruptible-eval/fn/fn
    interruptible_eval.clj:  190  clojure.tools.nrepl.middleware.interruptible-eval/run-next/fn
                  AFn.java:   22  clojure.lang.AFn/run
   ThreadPoolExecutor.java: 1142  java.util.concurrent.ThreadPoolExecutor/runWorker
   ThreadPoolExecutor.java:  617  java.util.concurrent.ThreadPoolExecutor$Worker/run
               Thread.java:  745  java.lang.Thread/run

If you call the exact same code, but with with-db-connection instead of with-db-transaction, then the exception you get is a java.net.SocketTimeoutException, which is the behaviour I would expect out of with-db-transaction as well.

I'm not at all familiar with the test cases for clojure.java.jdbc, so I'm not sure whether the above goes any way to helping out get automated coverage for this.

And yes – correct/safe error handling in JDBC is always a real nightmare. I suppose at least in Clojure we have a hope of abstracting that away reasonably, rather than the horrible copy/paste mess that it's so easy for equivalent Java projects to end up wallowing in!

Comment by Sean Corfield [ 31/May/16 12:34 PM ]

That's awesome Sam – thank you!





Generated at Tue May 31 15:05:29 CDT 2016 using JIRA 4.4#649-r158309.