From 3cf911b80127ccf5803085d8bcefe05c109e6086 Mon Sep 17 00:00:00 2001 From: Oleksandr Yakushev Date: Sat, 5 Oct 2024 12:12:32 +0300 Subject: [PATCH] [jdbc.read] Remove value caching from JDBC reading machinery --- src/toucan2/jdbc/read.clj | 74 +++++++-------------------------- src/toucan2/jdbc/result_set.clj | 8 ++-- 2 files changed, 18 insertions(+), 64 deletions(-) diff --git a/src/toucan2/jdbc/read.clj b/src/toucan2/jdbc/read.clj index a97e02e..4e09e7e 100644 --- a/src/toucan2/jdbc/read.clj +++ b/src/toucan2/jdbc/read.clj @@ -136,54 +136,17 @@ v (thunk)] (next.jdbc.rs/read-column-by-index v rsmeta i)))) -(defn- make-i->thunk [conn model rset] +(defn ^:no-doc make-i->thunk + "Given a connection `conn`, a `model` and a result set `rset`, return a function which given a column number `i` returns + a thunk that retrieves the column value of the current row from the result set." + [conn model rset] (comp (memoize (fn [i] (make-column-thunk conn model rset i))) int)) -(defn ^:no-doc make-cached-row-num->i->thunk - "Returns a function that, given the current row number, returns a function that, given a column number, returns a cached - thunk to fetch values of that column. Confusing, huh? Here's a chart to make it a bit easier to visualize: - - ```clj - (make-cached-row-num->i->thunk conn model rset) - => - (f current-row-number) - => - (f column-index) - => - (column-value-thunk) - ``` - - What's the point of this? The main point is to cache values that come out of the database, so we only fetch them once. - This is used to implement our transient rows in [[toucan2.jdbc.result-set]] -- accessing the value of a transient row - twice should not result in two calls to `.getObject`. - - The row number is used for cache-busting purposes, so we can reset the cache after each row is returned (so we don't - accidentally cache values from the first row and return them for all the rows). The row number passed in here doesn't - need to correspond to the actual row number from a JDBC standpoint; it's used only for cache-busting purposes." - [conn model ^ResultSet rset] - (let [i->thunk (make-i->thunk conn model rset) - cached-row-num (atom -1) - cached-values (atom {})] - (fn row-num->i->thunk* [current-row-num] - (when-not (= current-row-num @cached-row-num) - (reset! cached-row-num current-row-num) - (reset! cached-values {})) - (fn i->thunk* [i] - (fn cached-column-thunk [] - (let [cached-value (get @cached-values i ::not-found)] - (if-not (= cached-value ::not-found) - (log/tracef "Using cached value for column %s: %s" i cached-value) - cached-value) - (let [thunk (i->thunk i) - v (thunk)] - (swap! cached-values assoc i v) - v))))))) - (defn ^:no-doc read-column-by-index-fn - "Given a `java.sql.Connection` `conn`, a `model`, and a `java.sql.ResultSet` `rset`, return a function that can be used - with [[next.jdbc.result-set/builder-adapter]]. The function has the signature + "Given a `i->thunk` function, return a function that can be used with [[next.jdbc.result-set/builder-adapter]]. The + function has the signature ```clj (f builder rset i) => result @@ -193,19 +156,12 @@ index `i`, it will return the value of that column for the current row. The function used to fetch the column is built by combining [[read-column-thunk]] - and [[next.jdbc.result-set/read-column-by-index]]; the function is built once and used repeatedly for each new row. - - Values are cached for the current row -- fetching the same column twice for a given row will only result in fetching - it from the database once." - ([conn model ^ResultSet rset] - (read-column-by-index-fn (make-cached-row-num->i->thunk conn model rset))) - - ([row-num->i->thunk] - (fn read-column-by-index-fn* [_builder ^ResultSet rset ^Integer i] - (assert (not (.isClosed ^ResultSet rset)) - "ResultSet is already closed. Do you need call toucan2.realize/realize on the row before the Connection is closed?") - (let [i->thunk (row-num->i->thunk (.getRow rset)) - thunk (i->thunk i) - result (thunk)] - (log/tracef "col %s => %s" i result) - result)))) + and [[next.jdbc.result-set/read-column-by-index]]; the function is built once and used repeatedly for each new row." + [i->thunk] + (fn read-column-by-index-fn* [_builder ^ResultSet rset ^Integer i] + (assert (not (.isClosed ^ResultSet rset)) + "ResultSet is already closed. Do you need call toucan2.realize/realize on the row before the Connection is closed?") + (let [thunk (i->thunk i) + result (thunk)] + (log/tracef "col %s => %s" i result) + result))) diff --git a/src/toucan2/jdbc/result_set.clj b/src/toucan2/jdbc/result_set.clj index a5384cd..90dd8bb 100644 --- a/src/toucan2/jdbc/result_set.clj +++ b/src/toucan2/jdbc/result_set.clj @@ -131,10 +131,10 @@ directly." [rf init conn model ^ResultSet rset opts] (log/debugf "Reduce JDBC result set for model %s with rf %s and init %s" model rf init) - (let [row-num->i->thunk (jdbc.read/make-cached-row-num->i->thunk conn model rset) + (let [i->thunk (jdbc.read/make-i->thunk conn model rset) builder-fn* (next.jdbc.rs/builder-adapter (builder-fn conn model rset opts) - (jdbc.read/read-column-by-index-fn row-num->i->thunk)) + (jdbc.read/read-column-by-index-fn i->thunk)) builder (builder-fn* rset opts) combined-opts (jdbc.options/merge-options (merge (:opts builder) opts)) label-fn (get combined-opts :label-fn) @@ -151,9 +151,7 @@ (log/tracef "Result set has no more rows.") acc) - :let [row-num (.getRow rset) - _ (log/tracef "Fetch row %s" row-num) - i->thunk (row-num->i->thunk row-num) + :let [_ (log/tracef "Fetch row %s" (.getRow rset)) row (jdbc.row/row model rset builder i->thunk col-name->index) acc' (rf acc row)]