Skip to content

Commit

Permalink
[jdbc.read] Don't track realized keys in TransientRow
Browse files Browse the repository at this point in the history
  • Loading branch information
@alexander-yakushev
alexander-yakushev committed Oct 8, 2024
1 parent 352d181 commit 463e943
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 32 deletions.
44 changes: 24 additions & 20 deletions src/toucan2/jdbc/result_set.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,31 +67,34 @@
(rs! [_this acc]
(persistent! acc)))

(defn- column-name->keyword [column-name label-fn]
(when (or (string? column-name)
(instance? clojure.lang.Named column-name))
(keyword
(when (instance? clojure.lang.Named column-name)
(when-let [col-ns (namespace column-name)]
(name (label-fn (name col-ns)))))
(name (label-fn (name column-name))))))

(defn- make-column-name->index [cols label-fn]
{:pre [(fn? label-fn)]}
(if (empty? cols)
(constantly nil)
(memoize
(fn [column-name]
(when (or (string? column-name)
(instance? clojure.lang.Named column-name))
;; TODO FIXME -- it seems like the column name we get here has already went thru the label fn/qualifying
;; functions. The `(originally ...)` in the log message is wrong. Are we applying label function twice?!
(let [column-name' (keyword
(when (instance? clojure.lang.Named column-name)
(when-let [col-ns (namespace column-name)]
(label-fn (name col-ns))))
(label-fn (name column-name)))
i (when column-name'
(first (keep-indexed
(fn [i col]
(when (= col column-name')
(inc i)))
cols)))]
(log/tracef "Index of column named %s (originally %s) is %s" column-name' column-name i)
(when-not i
(log/debugf "Could not determine index of column name %s. Found: %s" column-name cols))
i))))))
;; TODO FIXME -- it seems like the column name we get here has already went thru the label fn/qualifying
;; functions. The `(originally ...)` in the log message is wrong. Are we applying label function twice?!
(let [column-name' (column-name->keyword column-name label-fn)
i (when column-name'
(first (keep-indexed
(fn [i col]
(when (= col column-name')
(inc i)))
cols)))]
(log/tracef "Index of column named %s (originally %s) is %s" column-name' column-name i)
(when-not i
(log/debugf "Could not determine index of column name %s. Found: %s" column-name cols))
i)))))

(defn instance-builder-fn
"Create a result set map builder function appropriate for passing as the `:builder-fn` option to `next.jdbc` that
Expand Down Expand Up @@ -142,6 +145,7 @@
col-names (get builder :cols (next.jdbc.rs/get-modified-column-names
(.getMetaData rset)
combined-opts))
col-names-kw (vec (keep #(column-name->keyword % label-fn) col-names))
col-name->index (make-column-name->index col-names label-fn)]
(log/tracef "column name -> index = %s" col-name->index)
(loop [acc init]
Expand All @@ -152,7 +156,7 @@
acc)

:let [_ (log/tracef "Fetch row %s" (.getRow rset))
row (jdbc.row/row model rset builder i->thunk col-name->index)
row (jdbc.row/row model rset builder i->thunk col-name->index col-names-kw)
acc' (rf acc row)]

(reduced? acc')
Expand Down
21 changes: 10 additions & 11 deletions src/toucan2/jdbc/row.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@
;; a function that given a column name key will normalize it and return the
;; corresponding JDBC index. This should probably be memoized for the whole result set.
column-name->index
;; an atom with a set of realized column name keywords.
realized-keys
;; a list of all column names
column-names
;; ATOM with map. Given a JDBC column index (starting at 1) return a thunk that can be
;; used to fetch the column. This usually comes
;; from [[toucan2.jdbc.read/make-cached-i->thunk]].
Expand Down Expand Up @@ -83,7 +83,6 @@
(pr-str transient-row')
(pr-str (.valAt transient-row' k))))
transient-row')))
(swap! realized-keys conj k)
this)))

;; TODO -- can we `assocEx` the transient row?
Expand All @@ -110,7 +109,6 @@
(if (= index k-index)
(constantly ::not-found)
(i->thunk index))))))
(swap! realized-keys disj k)
(if (identical? column-name->index column-name->index')
this
(TransientRow. model
Expand Down Expand Up @@ -285,11 +283,13 @@
[^toucan2.jdbc.row.TransientRow row]
(try
(let [transient-row @(.volatile_transient_row row)
realized-keys (.realized_keys row)]
column-names (.column_names row)]
[(symbol (format "^%s " `TransientRow))
;; (instance? pretty.core.PrettyPrintable transient-row) (pretty/pretty transient-row)
(zipmap @realized-keys
(map #(get transient-row %) @realized-keys))])
(into {} (keep (fn [col] (let [v (get transient-row col ::not-found)]
(when-not (= v ::not-found)
[col (get transient-row col)]))))
column-names)])
(catch Exception _
["unrealized result set {row} -- do you need to call toucan2.realize/realize ?"])))

Expand Down Expand Up @@ -346,18 +346,17 @@
(defn ^:no-doc row
"Create a new `TransientRow`. Part of the low-level implementation of the JDBC query execution backend. You probably
shouldn't be using this directly!"
[model ^ResultSet rset builder i->thunk col-name->index]
[model ^ResultSet rset builder i->thunk col-name->index column-names]
(assert (not (.isClosed rset)) "ResultSet is already closed")
(let [volatile-transient-row (volatile! (next.jdbc.rs/->row builder))
i->thunk (atom i->thunk)
realized-row-delay (make-realized-row-delay builder i->thunk volatile-transient-row)
realized-keys (atom #{})]
realized-row-delay (make-realized-row-delay builder i->thunk volatile-transient-row)]
;; this is a gross amount of positional args. But using `reify` makes debugging things too hard IMO.
(->TransientRow model
rset
builder
col-name->index
realized-keys
column-names
i->thunk
volatile-transient-row
realized-row-delay)))
6 changes: 5 additions & 1 deletion test/toucan2/jdbc/row_test.clj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@
(select/reducible-select ::test/venues 1)))))

(defn- realized-keys [^TransientRow row]
@(.realized_keys row))
(let [transient-row @(.volatile_transient_row row)
column-names (.column_names row)]
(set (filter #(let [v (get transient-row % ::not-found)]
(not= v ::not-found))
column-names))))

(defn- already-realized? [^TransientRow row]
(realized? (.realized_row row)))
Expand Down

0 comments on commit 463e943

Please sign in to comment.