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

Extend Executable to an arbitrary Connectable #189

Closed holyjak closed 2 years ago

holyjak commented 2 years ago

I have a custom Connectable implementation and I would like to use it with jdbc/execute!, which currently is not possible. I have to manually do (with-open [c (jdbc/get-connection my-connectable) (jdbc/execute! c [...])).

Since a DataSource is Connectable and the only think that the Executable implementation uses the D.S. for is jdbc/get-connection, we could just as well replace javax.sql.DataSource with next.jdbc.protocols.Connectable here:

https://github.com/seancorfield/next-jdbc/blob/d555d42e79b162bdd957b757c98bcfcfeecb9005/src/next/jdbc/result_set.clj#L882

and it would still work for data sources but also for any arbitrary connectable.

(At least in theory. I can imagine that practice is more complicated.)

holyjak commented 2 years ago

Ok, one option would be

(extend-protocol Executable
    Object ; default fallthrough
    (execute [this sql-params opts]
      (if (satisfies? this Connectable)
        (do (extend-protocol Executable
              (class this)
              (-execute [this sql-params opts] <paste javax.sql.DataSource's -execute impl.>))
            (-execute this sql-params opts))
        (assert false "Unhandled entity type")))
     ...)

another would be to modify next.jdbc.execute! and friends to handle Connectable:

(defn execute! [connectable sql-params opts]
  (if (satisfies? Connectable connectable)
      (with-open [conn (get-connection connectable)]
        (p/-execute-all conn sql-params
                        (assoc opts :next.jdbc/sql-params sql-params)))
      (p/-execute-all connectable sql-params
                      (assoc opts :next.jdbc/sql-params sql-params)))
)

I am not experimenting with this:

(defn extend-executable-to-connectable [connectable]
  (assert (satisfies? jdbc.p/Connectable connectable) "Cannot be converted to an Executable")
  (extend-protocol jdbc.p/Executable
    (class connectable)
    (-execute [this sql-params opts]
    (reify
      clojure.lang.IReduceInit
      (reduce [_ f init]
        (with-open [con  (jdbc.p/get-connection this opts)]
          (.reduce (jdbc/plan con sql-params opts) f init)))
      r/CollFold
      (coll-fold [_ n combinef reducef]
        (with-open [con  (jdbc/get-connection this opts)]
          (r/coll-fold (jdbc.p/-execute con sql-params opts) n combinef reducef)))
      (toString [_] "`IReduceInit` from `plan` -- missing reduction?")))
    (-execute-one [this sql-params opts]
      (with-open [c (jdbc.p/get-connection this opts)] (jdbc.p/-execute-one c sql-params opts)))
    (-execute-all [this sql-params opts]
      (with-open [c (jdbc.p/get-connection this opts)] (jdbc.p/-execute-all c sql-params opts)))))

(extend-protocol jdbc.p/Executable
  Object ; default fallthrough
  (-execute [this sql-params opts]
    (extend-executable-to-connectable this)
    (jdbc.p/-execute this sql-params opts))
  (-execute-one [this sql-params opts]
    (extend-executable-to-connectable this)
    (jdbc.p/-execute-one this sql-params opts))
  (-execute-all [this sql-params opts]
    (extend-executable-to-connectable this)
    (jdbc.p/-execute-all this sql-params opts)))
seancorfield commented 2 years ago

@holyjak Can you explain what your custom Connectable is and why you need it? Could it be Sourceable instead? Could you extend Executable to your custom thing?

holyjak commented 2 years ago

Sure!

I use a custom Connectable as a wrapper around a DataSource so that I can execute set role <role> when a connection is checked out from the underlying pool. I use this as it seemed simpler than making a proxy for DataSource (which I certainly could, and already do for a Connection) So no, it could not be Sourceable.

And yes, I can extend Executable, which I did:

(defrecord RoledBasedDs [^DataSource ds role]
  jdbc.p/Connectable
  (get-connection ^Connection [_ opts]
    (let [conn (jdbc/get-connection ds opts)]
      (jdbc/execute! conn [(format "set role %s; set schema '%s'" role role)])
      (RoleResettingConnection. conn))) ; a custom proxy of Connection

  jdbc.p/Executable
  (-execute [this sql-params opts]
    (reify
      clojure.lang.IReduceInit
      (reduce [_ f init]
        (with-open [con  (jdbc.p/get-connection this opts)]
          (.reduce (jdbc/plan con sql-params opts) f init)))
      r/CollFold
      (coll-fold [_ n combinef reducef]
        (with-open [con  (jdbc/get-connection this opts)]
          (r/coll-fold (jdbc.p/-execute con sql-params opts) n combinef reducef)))
      (toString [_] "`IReduceInit` from `plan` -- missing reduction?")))
  (-execute-one [this sql-params opts]
    (with-open [c (jdbc.p/get-connection this opts)] (jdbc.p/-execute-one c sql-params opts)))
  (-execute-all [this sql-params opts]
    (with-open [c (jdbc.p/get-connection this opts)] (jdbc.p/-execute-all c sql-params opts)))

  jdbc.p/Transactable
  (-transact [this body-fn opts]
    (with-open [c (jdbc.p/get-connection this opts)] (jdbc.p/-transact c body-fn opts))))

So it is certainly possible and the current situation is no "blocker". On the other hand it was not trivial to find out how to implement it (while delegating most work back to next-jdbc).

seancorfield commented 2 years ago

How about implementing Sourceable as just returning this which has a precedent:

(extend-protocol p/Sourceable
  clojure.lang.Associative
  (get-datasource [this]
                  (url+etc->datasource (spec->url+etc this)))
  javax.sql.DataSource
  (get-datasource [this] this)
  String
  (get-datasource [this]
                  (url+etc->datasource (string->url+etc this))))

This would obviate the need for all the executable/transactable implementations.

seancorfield commented 2 years ago

I've given this a lot of thought and poked and prodded at the code and extending the existing protocols is fraught with edge cases so I'm not going to do that.

My recommendation -- and there is precedent in next.jdbc already for this -- is to reify javax.sql.DataSource (not proxy) since it has a very small surface area to implement, and it implements getConnection which is what you want to hook into.

holyjak commented 2 years ago

Thank you!