seancorfield / next-jdbc

A modern low-level Clojure wrapper for JDBC-based access to databases.
https://cljdoc.org/d/com.github.seancorfield/next.jdbc/
Eclipse Public License 1.0
755 stars 90 forks source link

with-transaction doesn't respect :builder-fn? #205

Closed tomekw closed 2 years ago

tomekw commented 2 years ago

Describe the bug with-transaction doesn't respect :builder-fn next.jdbc.result-set/as-kebab-maps

I've db defined as:

(def ^:private db
  (-> db-spec
      (jdbc/get-datasource)
      (jdbc/with-options {:builder-fn rs/as-kebab-maps})))

To Reproduce

(jdbc/execute! db (sql/format {:select [:id :current_day]
                               :from [:authors]}))
;; => [#:authors{:id "7813a5f8-9cad-48d8-bd78-d95c95bc184e",
;;               :current-day "2022-05-02"}]

(jdbc/with-transaction [tx db]
  (jdbc/execute! tx (sql/format {:select [:id :current_day]
                                 :from [:authors]})))
;; => [#:authors{:id "7813a5f8-9cad-48d8-bd78-d95c95bc184e",
;;               :current_day "2022-05-02"}]

Expected behavior

;; => [#:authors{:id "7813a5f8-9cad-48d8-bd78-d95c95bc184e",
;;               :current-day "2022-05-02"}]

project.clj/deps.edn

        org.clojure/clojure {:mvn/version "1.11.1"}
        com.github.seancorfield/honeysql {:mvn/version "2.2.868"}
        com.github.seancorfield/next.jdbc {:mvn/version "1.2.780"}
        org.xerial/sqlite-jdbc {:mvn/version "3.36.0.3"}

Environment (please complete the following information):

seancorfield commented 2 years ago

That is specifically called out as a caveat in the documentation: https://cljdoc.org/d/com.github.seancorfield/next.jdbc/1.2.780/doc/getting-started#datasources-connections--transactions

Since next.jdbc using raw Java objects for Connection etc, you have to explicitly re-wrap them yourself if you want options to carry across with-transaction.

A while back, I spent a fair bit of time looking into the possibility of having with-transaction automatically rewrap the options if it was asked to create a Connection with a transaction from a wrapped datasource or connection and concluded it wasn't realistic to do it automatically: https://github.com/seancorfield/next-jdbc/issues/172

tomekw commented 2 years ago

Oh, my bad!

Especially when I tweeted this today in the morning:

https://twitter.com/_tomekw/status/1522490350878797824

🤦

Thank you!