datafy incorrect on metadata result sets for sqlite

flyingmachine commented 2 years ago

Describe the bug

Calling (clojure.datafy/datafy (-> conn .getMetaData (.getTables nil nil nil nil))) returns column information for the results rather than the results themselves

To Reproduce

Run the following:

(with-open [conn (next.jdbc/get-connection {:dbtype "sqlite" :dbname "sqlite.db"})]
  (->
   (.getMetaData conn)
   (.getTables nil nil nil nil)
   clojure.datafy/datafy))

This returns:

[{:schema "",
  :table "",
  :scale 0,
  :name "TABLE_CAT",
  :precision 0,
  :type "NUMERIC",
  :writable true,
  :nullability :null,
  :searchable true,
  :currency false,
  :definitely-writable true,
  :label "TABLE_CAT",
  :class "java.lang.Object",
  :case-sensitive true,
  :read-only false,
  :auto-increment false,
  :catalog nil,
  :display-size 2147483647,
  :signed true}
 {:schema "",
  :table "",
  :scale 0,
  :name "TABLE_SCHEM",
  :precision 0,
  :type "NUMERIC",
  :writable true,
  :nullability :null,
  :searchable true,
  :currency false,
  :definitely-writable true,
  :label "TABLE_SCHEM",
  :class "java.lang.Object",
  :case-sensitive true,
  :read-only false,
  :auto-increment false,
  :catalog nil,
  :display-size 2147483647,
  :signed true}
 {:schema "",
  :table "sqlite_master",
  :scale 0,
  :name "TABLE_NAME",
  :precision 0,
  :type "TEXT",
  :writable true,
  :nullability :null,
  :searchable true,
  :currency false,
  :definitely-writable true,
  :label "TABLE_NAME",
  :class "java.lang.Object",
  :case-sensitive true,
  :read-only false,
  :auto-increment false,
  :catalog "sqlite_master",
  :display-size 2147483647,
  :signed false}
 {:schema "",
  :table "",
  :scale 0,
  :name "TABLE_TYPE",
  :precision 0,
  :type "TEXT",
  :writable true,
  :nullability :null,
  :searchable true,
  :currency false,
  :definitely-writable true,
  :label "TABLE_TYPE",
  :class "java.lang.Object",
  :case-sensitive true,
  :read-only false,
  :auto-increment false,
  :catalog nil,
  :display-size 2147483647,
  :signed false}
 {:schema "",
  :table "",
  :scale 0,
  :name "REMARKS",
  :precision 0,
  :type "NUMERIC",
  :writable true,
  :nullability :null,
  :searchable true,
  :currency false,
  :definitely-writable true,
  :label "REMARKS",
  :class "java.lang.Object",
  :case-sensitive true,
  :read-only false,
  :auto-increment false,
  :catalog nil,
  :display-size 2147483647,
  :signed true}
 {:schema "",
  :table "",
  :scale 0,
  :name "TYPE_CAT",
  :precision 0,
  :type "NUMERIC",
  :writable true,
  :nullability :null,
  :searchable true,
  :currency false,
  :definitely-writable true,
  :label "TYPE_CAT",
  :class "java.lang.Object",
  :case-sensitive true,
  :read-only false,
  :auto-increment false,
  :catalog nil,
  :display-size 2147483647,
  :signed true}
 {:schema "",
  :table "",
  :scale 0,
  :name "TYPE_SCHEM",
  :precision 0,
  :type "NUMERIC",
  :writable true,
  :nullability :null,
  :searchable true,
  :currency false,
  :definitely-writable true,
  :label "TYPE_SCHEM",
  :class "java.lang.Object",
  :case-sensitive true,
  :read-only false,
  :auto-increment false,
  :catalog nil,
  :display-size 2147483647,
  :signed true}
 {:schema "",
  :table "",
  :scale 0,
  :name "TYPE_NAME",
  :precision 0,
  :type "NUMERIC",
  :writable true,
  :nullability :null,
  :searchable true,
  :currency false,
  :definitely-writable true,
  :label "TYPE_NAME",
  :class "java.lang.Object",
  :case-sensitive true,
  :read-only false,
  :auto-increment false,
  :catalog nil,
  :display-size 2147483647,
  :signed true}
 {:schema "",
  :table "",
  :scale 0,
  :name "SELF_REFERENCING_COL_NAME",
  :precision 0,
  :type "NUMERIC",
  :writable true,
  :nullability :null,
  :searchable true,
  :currency false,
  :definitely-writable true,
  :label "SELF_REFERENCING_COL_NAME",
  :class "java.lang.Object",
  :case-sensitive true,
  :read-only false,
  :auto-increment false,
  :catalog nil,
  :display-size 2147483647,
  :signed true}
 {:schema "",
  :table "",
  :scale 0,
  :name "REF_GENERATION",
  :precision 0,
  :type "NUMERIC",
  :writable true,
  :nullability :null,
  :searchable true,
  :currency false,
  :definitely-writable true,
  :label "REF_GENERATION",
  :class "java.lang.Object",
  :case-sensitive true,
  :read-only false,
  :auto-increment false,
  :catalog nil,
  :display-size 2147483647,
  :signed true}]

Expected behavior

The above should return:

{:open true,
 :statement
 #object[org.sqlite.jdbc4.JDBC4Statement 0x2a6bf088 "org.sqlite.jdbc4.JDBC4Statement@2a6bf088"],
 :closed false,
 :holdability 0,
 :concurrency 1007,
 :beforeFirst true,
 :cursorName nil,
 :type 1003,
 :fetchSize 0,
 :warnings nil,
 :last/exception #error {
                         :cause "function not yet implemented for SQLite"
                         :via
                         [{:type java.lang.reflect.InvocationTargetException
                           :message nil
                           :at [jdk.internal.reflect.NativeMethodAccessorImpl invoke0 "NativeMethodAccessorImpl.java" -2]}
                          {:type java.sql.SQLException
                           :message "function not yet implemented for SQLite"
                           :at [org.sqlite.jdbc3.JDBC3ResultSet isLast "JDBC3ResultSet.java" 158]}]
                         :trace
                         [[org.sqlite.jdbc3.JDBC3ResultSet isLast "JDBC3ResultSet.java" 158]
                          [jdk.internal.reflect.NativeMethodAccessorImpl invoke0 "NativeMethodAccessorImpl.java" -2]
                          [jdk.internal.reflect.NativeMethodAccessorImpl invoke "NativeMethodAccessorImpl.java" 62]
                          [jdk.internal.reflect.DelegatingMethodAccessorImpl invoke "DelegatingMethodAccessorImpl.java" 43]
                          [java.lang.reflect.Method invoke "Method.java" 566]
                          [clojure.java.data$make_shallow_getter_fn$fn__7935 invoke "data.clj" 98]
                          [clojure.java.data$eval8106$fn__8107$iter__8108__8112$fn__8113$fn__8132 invoke "data.clj" 351]
                          [clojure.java.data$eval8106$fn__8107$iter__8108__8112$fn__8113 invoke "data.clj" 350]
                          [clojure.lang.LazySeq sval "LazySeq.java" 42]
                          [clojure.lang.LazySeq seq "LazySeq.java" 51]
                          [clojure.lang.Cons next "Cons.java" 39]
                          [clojure.lang.RT next "RT.java" 713]
                          [clojure.core$next__5451 invokeStatic "core.clj" 64]
                          [clojure.core.protocols$fn__8249 invokeStatic "protocols.clj" 169]
                          [clojure.core.protocols$fn__8249 invoke "protocols.clj" 124]
                          [clojure.core.protocols$fn__8204$G__8199__8213 invoke "protocols.clj" 19]
                          [clojure.core.protocols$seq_reduce invokeStatic "protocols.clj" 31]
                          [clojure.core.protocols$fn__8236 invokeStatic "protocols.clj" 75]
                          [clojure.core.protocols$fn__8236 invoke "protocols.clj" 75]
                          [clojure.core.protocols$fn__8178$G__8173__8191 invoke "protocols.clj" 13]
                          [clojure.core$reduce invokeStatic "core.clj" 6886]
                          [clojure.core$into invokeStatic "core.clj" 6958]
                          [clojure.core$into invoke "core.clj" 6950]
                          [clojure.java.data$eval8106$fn__8107 invoke "data.clj" 365]
                          [clojure.lang.MultiFn invoke "MultiFn.java" 234]
                          [next.jdbc.datafy$safe_bean invokeStatic "datafy.clj" 94]
                          [next.jdbc.datafy$safe_bean invoke "datafy.clj" 92]
                          [clojure.lang.Var invoke "Var.java" 388]
                          [donut.dbxray$eval11788$fn__11789 invoke "NO_SOURCE_FILE" 42]
                          [clojure.core.protocols$fn__8277$G__8272__8282 invoke "protocols.clj" 182]
                          [clojure.datafy$datafy invokeStatic "datafy.clj" 23]
                          [clojure.datafy$datafy invoke "datafy.clj" 15]
                          [donut.dbxray_test$eval11793 invokeStatic "NO_SOURCE_FILE" 206]
                          [donut.dbxray_test$eval11793 invoke "NO_SOURCE_FILE" 205]
                          [clojure.lang.Compiler eval "Compiler.java" 7194]
                          [clojure.lang.Compiler eval "Compiler.java" 7149]
                          [clojure.core$eval invokeStatic "core.clj" 3215]
                          [clojure.core$eval invoke "core.clj" 3211]
                          [nrepl.middleware.interruptible_eval$evaluate$fn__1245$fn__1246 invoke "interruptible_eval.clj" 87]
                          [clojure.lang.AFn applyToHelper "AFn.java" 152]
                          [clojure.lang.AFn applyTo "AFn.java" 144]
                          [clojure.core$apply invokeStatic "core.clj" 667]
                          [clojure.core$with_bindings_STAR_ invokeStatic "core.clj" 1990]
                          [clojure.core$with_bindings_STAR_ doInvoke "core.clj" 1990]
                          [clojure.lang.RestFn invoke "RestFn.java" 425]
                          [nrepl.middleware.interruptible_eval$evaluate$fn__1245 invoke "interruptible_eval.clj" 87]
                          [clojure.main$repl$read_eval_print__9206$fn__9209 invoke "main.clj" 437]
                          [clojure.main$repl$read_eval_print__9206 invoke "main.clj" 437]
                          [clojure.main$repl$fn__9215 invoke "main.clj" 458]
                          [clojure.main$repl invokeStatic "main.clj" 458]
                          [clojure.main$repl doInvoke "main.clj" 368]
                          [clojure.lang.RestFn invoke "RestFn.java" 1523]
                          [nrepl.middleware.interruptible_eval$evaluate invokeStatic "interruptible_eval.clj" 84]
                          [nrepl.middleware.interruptible_eval$evaluate invoke "interruptible_eval.clj" 56]
                          [nrepl.middlew;; =>
                           are.interruptible_eval$interruptible_eval$fn__1278$fn__1282 invoke "interruptible_eval.clj" 152]
                          [clojure.lang.AFn run "AFn.java" 22]
                          [nrepl.middleware.session$session_exec$main_loop__1348$fn__1352 invoke "session.clj" 218]
                          [nrepl.middleware.session$session_exec$main_loop__1348 invoke "session.clj" 217]
                          [clojure.lang.AFn run "AFn.java" 22]
                          [java.lang.Thread run "Thread.java" 829]]},
 :rows
 [{:REF_GENERATION nil,
   :SELF_REFERENCING_COL_NAME nil,
   :sqlite_master/TABLE_NAME "todo_lists",
   :TABLE_CAT nil,
   :TYPE_CAT nil,
   :TYPE_SCHEM nil,
   :REMARKS nil,
   :TYPE_NAME nil,
   :TABLE_SCHEM nil,
   :TABLE_TYPE "TABLE"}
  {:REF_GENERATION nil,
   :SELF_REFERENCING_COL_NAME nil,
   :sqlite_master/TABLE_NAME "todos",
   :TABLE_CAT nil,
   :TYPE_CAT nil,
   :TYPE_SCHEM nil,
   :REMARKS nil,
   :TYPE_NAME nil,
   :TABLE_SCHEM nil,
   :TABLE_TYPE "TABLE"}
  {:REF_GENERATION nil,
   :SELF_REFERENCING_COL_NAME nil,
   :sqlite_master/TABLE_NAME "users",
   :TABLE_CAT nil,
   :TYPE_CAT nil,
   :TYPE_SCHEM nil,
   :REMARKS nil,
   :TYPE_NAME nil,
   :TABLE_SCHEM nil,
   :TABLE_TYPE "TABLE"}],
 :afterLast false,
 :class org.sqlite.jdbc4.JDBC4ResultSet,
 :columnCount 10,
 :first false,
 :metaData
 #object[org.sqlite.jdbc4.JDBC4ResultSet 0x7bac129f "org.sqlite.jdbc4.JDBC4ResultSet@7bac129f"],
 :fetchDirection 1000,
 :row 0}

Stacktraces

NA

project.clj/deps.edn

{:paths ["src"]
 :deps  {com.github.seancorfield/next.jdbc {:mvn/version "1.2.737"}}

 :aliases
 {:test
  {:extra-paths ["test"]
   :extra-deps  {org.clojure/test.check               {:mvn/version "0.9.0"}
                 io.github.cognitect-labs/test-runner {:git/tag "v0.5.0" :git/sha "48c3c67"}

                 ;; next-jdbc
                 io.zonky.test.postgres/embedded-postgres-binaries-darwin-amd64 {:mvn/version "13.4.0"}
                 io.zonky.test.postgres/embedded-postgres-binaries-linux-amd64  {:mvn/version "13.4.0"}
                 io.zonky.test/embedded-postgres                                {:mvn/version "1.3.1"}
                 org.postgresql/postgresql                                      {:mvn/version "42.3.1"}
                 org.xerial/sqlite-jdbc                                         {:mvn/version "3.36.0.3"}}
   :exec-fn cognitect.test-runner.api/test}

  :build
  {:deps       {io.github.seancorfield/build-clj
                {:git/tag "v0.6.6" :git/sha "171d5f1"}}
   :ns-default build}}}

Environment (please complete the following information):

OS: macOS
Java Version: openjdk version "11.0.11" 2021-04-20
Clojure Version: 1.11.1.149
Database: sqlite 3.37.0 2021-12-09 01:34:53 9ff244ce0739f8ee52a3e9671adb4ee54c83c640b02e3f9d185fd2f9a179aapl
Driver Library Version: org.xerial/sqlite-jdbc {:mvn/version "3.36.0.3"}

Additional context

The following fixes this problem:

(extend-protocol p/Datafiable
  org.sqlite.jdbc4.JDBC4ResultSet
  (datafy [this]
    (let [s (.getStatement this)
          c (when s (.getConnection s))]
      (cond-> (#'next.jdbc.datafy/safe-bean this {})
        c (assoc :rows (rs/datafiable-result-set this c {}))))))

seancorfield commented 2 years ago

Interesting. I'll add a note to the documentation about this -- that extension would have to be in user-space, where an SQLite dependency of whatever version has been added.

seancorfield commented 2 years ago

The reason for this weirdness is that SQLite has a strange combination ResultSet/Metadata object and next.jdbc already has a case for that:


  ResultSet
  (datafy [this]
    ;; SQLite has a combination ResultSet/Metadata object...
    (if (instance? ResultSetMetaData this)
      (datafy-result-set-meta-data this)
      (let [s (.getStatement this)
            c (when s (.getConnection s))]
        (cond-> (safe-bean this {})
          c (assoc :rows (rs/datafiable-result-set this c {}))))))

Without that special case, quite a few tests fail -- basically anywhere that tries to datafy metadata instead of result sets. I'm going to have to do some testing that adding this extension doesn't break other things with SQLite.

seancorfield commented 2 years ago

Yup, adding this extension breaks tests which means it breaks some of how things are supposed to work. I'll have to dig into this some more to establish how to make this case work without breaking other things.

flyingmachine commented 2 years ago

Seems like a tricky one!

flyingmachine commented 2 years ago

I've been noodling on how to make this work and I honestly can't think of something that makes sense. If I'm understanding this correctly, SQLite's implementation makes it impossible to distinguish cases where you want ResultSet behavior for datafy from those where you want ResultSetMetaData?

For my use case, it looks like using next.jdbc.result-set/datafiable-result-set directly works well enough.

seancorfield commented 2 years ago

@flyingmachine Yup, unfortunately, SQLite's implementation just makes some things ambiguous as far as selecting the intended behavior there.

seancorfield commented 2 years ago

@flyingmachine See https://github.com/xerial/sqlite-jdbc/blob/master/src/main/java/org/sqlite/jdbc4/JDBC4ResultSet.java#L30 (and note the 47 SQLFeatureNotSupportedException cases!).

seancorfield / next-jdbc

datafy incorrect on metadata result sets for sqlite #212