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
768 stars 90 forks source link

Make next.jdbc.result-set/row-builder public #173

Closed kylecbrodie closed 3 years ago

kylecbrodie commented 3 years ago

so users can use the RowBuilder objects returned by the other public functions in this namespace like as-unqualified-kebab-maps against a ResultSet object like one from a PreparedStatement as returned by next.jdbc/prepare

seancorfield commented 3 years ago

Can you explain the specific problem that this solves? So far no one has expressed an interest in using row-builder directly because the available extension points have been sufficient. What real-world problem can you not solve while row-builder is private?

Currently, row-builder is an implementation detail that I can change at will without breaking anyone's code.

kylecbrodie commented 3 years ago

Getting a lazy seq of rows like clojure.core/resultset-seq but using the next.jdbc extension points like the protocols RowBuilder and ReadableColumn and the column-reader function passed to as-maps-adapter. Here is an example,

(ns user
  (:require
   [clojure.java.io :as io]
   [excel-clj.core :as excel]
   [next.jdbc :as jdbc]
   [next.jdbc.result-set :as rs])
  (:import
   [java.sql PreparedStatement ResultSet]))

;; private function from next.jdbc.result-set
(defn- row-builder
  "Given a `RowBuilder` -- a row materialization strategy -- produce a fully
  materialized row from it."
  [builder]
  (->> (reduce (fn [r i] (rs/with-column builder r i))
               (rs/->row builder)
               (range 1 (inc (rs/column-count builder))))
       (rs/row! builder)))

(defn seq-resultset
  "Returns a lazy sequence of rows, built by builder-fn, from the ResultSet rs.
   It is the callers responsibility to ensure that rs remains open for the
   lifetime of the lazy sequence."
  ([^ResultSet rs] (seq-resultset rs {}))
  ([^ResultSet rs {:keys [builder-fn] :or {builder-fn rs/as-maps}}]
   (let [builder (delay (builder-fn rs opts))
         rows (fn thisfn []
                (when (.next rs)
                  (cons (row-builder @builder) (lazy-seq (thisfn)))))]
     (rows))))

(with-open [fos (io/output-stream "./file.xlsx")
            conn (jdbc/get-connection ds)
            ^PreparedStatement stmt (jdbc/prepare
                                     conn
                                     ["SELECT * FROM table"]
                                     jdbc/unqualified-snake-kebab-opts)]
  (when (.execute stmt)
    (excel/write-stream!
     {"Worksheet Name"
      (excel/table-grid
       (seq-resultset
        (.getResultSet stmt)
        jdbc/unqualified-snake-kebab-opts))}
     fos
     {:auto-size-cols? true})))

The Excel library can consume a sequence but it doesn't have functions that could reduce a reducible like those returned by jdbc/plan. I could also change this PR to add seq-resultset instead of making public row-builder

seancorfield commented 3 years ago

Lazy sequences are dangerous around resource management. That's why next.jdbc only offers fully-realized sequences (via execute! etc) or a reduce-based workflow via plan. clojure.java.jdbc allowed lazy sequences to be used and people constantly shot themselves in the foot with it. I'm not making row-builder public to support a workflow like this.

kylecbrodie commented 3 years ago

Makes sense. I very much appreciate Clojure and third party libraries eliminating classes of bugs that are common when working in Java. Out of curiosity, why is row-builder private? It seems like row-builder is the canonical way of using an object that implements the RowBuilder protocol and the protocol is public. How would row-builder change in a meaningful way without the public RowBuilder protocol also changing?

seancorfield commented 3 years ago

See https://github.com/seancorfield/next-jdbc/blob/develop/test/next/jdbc/result_set_test.clj#L384-L406 as an example of implementing the protocols without row-builder.

row-builder drives the protocols but is itself opaque -- and the whole trick with the delay of the builder-fn call is absolutely an implementation detail.

What you might consider is using reduce over plan to push datafiable-row results onto a core.async channel and then have a lazy sequence that consumes that channel, which you consume via excel/write-stream!. However, you still have a potential resource issue if you don't consume the entire sequence (perhaps you crash before it completes, leaving the reduce hanging) and you would need to take care to close! the channel outside the reduce once it has completed so you don't leave the lazy sequence hanging either.

seancorfield commented 3 years ago

I nearly suggested just calling datafiable-row on the ResultSet in my previous comment but realized that wasn't quite possible -- so I have extended the DatafiableRow protocol to ResultSet now (on develop).

That would allow you to write the above as:

(defn seq-resultset
  "Returns a lazy sequence of rows, built by builder-fn, from the ResultSet rs.
   It is the callers responsibility to ensure that rs remains open for the
   lifetime of the lazy sequence."
  ([^ResultSet rs] (seq-resultset rs {}))
  ([^ResultSet rs opts]
   (let [rows (fn thisfn []
                (when (.next rs)
                  (cons (rs/datafiable-row rs nil opts) (lazy-seq (thisfn)))))]
     (rows))))

No need for row-builder at that point.

Let me know if that works for you. Yes, it makes a new builder for each row so, yes, it remakes the columns for each row, but you're already in an edge case situation where you're working with ResultSet external to next.jdbc so that small extra overhead probably doesn't matter too much for the convenience you get?

kylecbrodie commented 3 years ago

Let me know if that works for you.

Yes it does. Thank you!