korma / Korma

Tasty SQL for Clojure.
http://sqlkorma.com
1.47k stars 222 forks source link

multiple belongs-to generates invalid sqlite3 query, but ok dry-run output #311

Closed dsvensson closed 9 years ago

dsvensson commented 9 years ago

Having multiple belongs-to generates invalid query, the table that has the belongs-to will not be found.

(defentity baz1
  (table :baz :baz1)
  (pk :baz_id))

(defentity baz2
  (table :baz :baz2)
  (pk :baz_id))

(defentity bar
  (pk :bar_id))

(defentity foo
  (pk :foo_id)
  (belongs-to bar {:fk :bar_id})
  (belongs-to baz1 {:fk :baz1_id})
  (belongs-to baz2 {:fk :baz2_id}))

;; Will crash with 'no such table: foo' when preparing the
;; statement within sqlite3's native code
(select foo
         (with bar (fields :bar_name))
         (with baz1 (fields [:baz_name :baz1_name]))
         (with baz2 (fields [:baz_name :baz2_name])))

Trace:

Caused by: java.sql.SQLException: no such table: foo
at org.sqlite.DB.throwex(DB.java:288)
at org.sqlite.NativeDB.prepare(Native Method)
at org.sqlite.DB.prepare(DB.java:114)
at org.sqlite.PrepStmt.<init>(PrepStmt.java:37)
at org.sqlite.Conn.prepareStatement(Conn.java:231)
at org.sqlite.Conn.prepareStatement(Conn.java:224)
at org.sqlite.Conn.prepareStatement(Conn.java:213)
at com.mchange.v2.c3p0.impl.NewProxyConnection.prepareStatement(NewProxyConnection.java:213)
at clojure.java.jdbc$prepare_statement.doInvoke(jdbc.clj:457)
at clojure.lang.RestFn.invoke(RestFn.java:425)
at clojure.lang.AFn.applyToHelper(AFn.java:156)
at clojure.lang.RestFn.applyTo(RestFn.java:132)
at clojure.core$apply.invoke(core.clj:628)
at clojure.java.jdbc$db_query_with_resultset.invoke(jdbc.clj:796)
at clojure.java.jdbc$query.doInvoke(jdbc.clj:832)
at clojure.lang.RestFn.invoke(RestFn.java:464)
at korma.db$exec_sql.invoke(db.clj:260)
at korma.db$do_query.invoke(db.clj:277)
at korma.core$exec.invoke(core.clj:498)
at korma_bug.core$_main.doInvoke(core.clj:38)

Full code example to reproduce the bug can be found here: https://github.com/dsvensson/korma-bug

Hopefully I'm the one at fault here, but I can't seem to get it working. Weird thing though is that (dry-run) of the statement prints valid sqlite3 code that can be executed in the sqlite3 cli.

dsvensson commented 9 years ago

The problem seems to be caused by a heap of () being inserted in the query. If I execute the dry-run output of the query above via (exec-raw ..):

SELECT
    "foo".*, "bar"."bar_name", 
    "baz1"."baz_name" AS "baz1_name", 
    "baz2"."baz_name" AS "baz2_name"
        FROM (("foo" LEFT JOIN "bar" ON "bar"."bar_id" = "foo"."bar_id") 
          LEFT JOIN "baz" AS "baz1" ON "baz1"."baz_id" = "foo"."baz1_id")
          LEFT JOIN "baz" AS "baz2" ON "baz2"."baz_id" = "foo"."baz2_id"

I get the same error, "no such table: foo", but if I remove the parens it works:

SELECT
    "foo".*, "bar"."bar_name", 
    "baz1"."baz_name" AS "baz1_name", 
    "baz2"."baz_name" AS "baz2_name"
        FROM "foo" LEFT JOIN "bar" ON "bar"."bar_id" = "foo"."bar_id"
          LEFT JOIN "baz" AS "baz1" ON "baz1"."baz_id" = "foo"."baz1_id"
          LEFT JOIN "baz" AS "baz2" ON "baz2"."baz_id" = "foo"."baz2_id"

So perhaps different scoping when pasted into the sqlite3 cli compared to PrepStmt? Why are those parenthesis added there in the first place?

dsvensson commented 9 years ago

Adding the following monkey-patch "fixes" the problem, alarm bells are ringing though since the grouping might actually make sense in some cases (although my SQL-fu is too weak to know in what cases):

(in-ns 'korma.sql.utils)
(defn left-assoc [vs]
  (loop [ret "" [v & vs] vs]
    (cond
      (nil? v) ret
      (nil? vs) (str ret v)
      :else (recur (str ret v) vs)))) ;; used to pass (str ret v) via (wrap ..) before.
(in-ns 'my.namespace)
immoh commented 9 years ago

I cloned your repo and it works fine for me without any modifications (monkey-patch is commented out).

Looks like parentheses were added because it was not working on MS Access: https://github.com/korma/Korma/commit/16645d4116f0e6848aafb8d6e28af0f4bd7a7aea

dsvensson commented 9 years ago

Huh.. weird.. the last output when I clone and run it without commenting out the monkeypatch is:

adding any combination of two or more belongs-to links fails, but dry-run looks fine and works in sqlite3 cli:

running:
dry run :: SELECT "foo".*, "bar"."bar_name", "baz1"."baz_name" AS "baz1_name", "baz2"."baz_name" AS "baz2_name" FROM (("foo" LEFT JOIN "bar" ON "bar"."bar_id" = "foo"."bar_id") LEFT JOIN "baz" AS "baz1" ON "baz1"."baz_id" = "foo"."baz1_id") LEFT JOIN "baz" AS "baz2" ON "baz2"."baz_id" = "foo"."baz2_id" :: []
[{:bar_id 1, :baz1_id 1, :baz2_id 1, :foo_id 1}]
now:

bug triggered
 no such table: foo%                                                                                                                             
immoh commented 9 years ago

Which sqlite3 version are you using?

I tested with 3.7.15.2 2013-01-09 11:53:05 c0e09560d26f0a6456be9dd3447f5311eb4f238f on OS X.

dsvensson commented 9 years ago

sqlitejdbc-0.5.6.jar, which contains the sqlite library I assume? As mentioned, running the command in the sqlite cli works just fine. Browsing the output of "strings linux-amd64.lib" tells me that the version of the sqlite library embedded in the jar file is 3.6.3. If that is not the library loaded when running, then the system wide version is 3.8.10.2, Fedora F22.

juhovh commented 9 years ago

Cannot reproduce on OS X either, and I added one "SELECT sqlite_version()" call to the code so that it reports the actual used library version, which is "3.6.14.2" for me.

dsvensson commented 9 years ago

@juhovh My output is: ({:sqlite_version() 3.6.3})

dsvensson commented 9 years ago

SQLite-bug confirmed, updating the jdbc driver fixes the problem,

-                 [sqlitejdbc "0.5.6"]
+                 [org.xerial/sqlite-jdbc "3.8.10.1"]

Perhaps a list of recommended drivers for the various databases would be a nice additions.

Broken one from: https://clojars.org/sqlitejdbc