From 35fc00d4b36ce1ad2ba7c3e28bf117186dfebd0b Mon Sep 17 00:00:00 2001 From: Sean Corfield Date: Sun, 7 Jun 2020 09:39:04 -0700 Subject: [PATCH 1/4] Fixes #119 by improving docstrings --- CHANGELOG.md | 1 + src/next/jdbc/result_set.clj | 23 +++++++++++++++-------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d87b06b..6fd2cca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ Only accretive/fixative changes will be made from now on. Changes made on master since 1.0.462: * Add tests for `"jtds"` database driver (against MS SQL Server), making it officially supported. * Switch from OpenTable Embedded PostgreSQL to Zonky's version, so that testing can move forward from PostgreSQL 10.11 to 12.2.0. +* Address #119 by clarifying realization actions in the docstrings for `row-number`, `column-names`, and `metadata`. * Add log4j2 as a test dependency so that I have better control over logging (which makes debugging easier!). ## Stable Builds diff --git a/src/next/jdbc/result_set.clj b/src/next/jdbc/result_set.clj index 13b7b36..c2aef20 100644 --- a/src/next/jdbc/result_set.clj +++ b/src/next/jdbc/result_set.clj @@ -396,16 +396,23 @@ to realize a row by calling `datafiable-row` but still wants to call these functions on the (realized) row." (row-number [this] - "Return the current 1-based row number, if available.") + "Return the current 1-based row number, if available. + + Should not cause any row realization.") (column-names [this] - "Return a vector of the column names from the result set.") + "Return a vector of the column names from the result set. + + Reifies the result builder, in order to construct column names, + but should not cause any row realization.") (metadata [this] "Return the raw `ResultSetMetaData` object from the result set. - If `next.jdbc.datafy` has been required, this will be fully-realized - as a Clojure data structure, otherwise this should not be allowed to - 'leak' outside of the reducing function as it may depend on the - connection remaining open, in order to be valid.")) + Should not cause any row realization. + + If `next.jdbc.datafy` has been required, this metadata will be + fully-realized as a Clojure data structure, otherwise this should + not be allowed to 'leak' outside of the reducing function as it may + depend on the connection remaining open, in order to be valid.")) (defn- mapify-result-set "Given a `ResultSet`, return an object that wraps the current row as a hash @@ -428,7 +435,7 @@ InspectableMapifiedResultSet (row-number [this] (.getRow rs)) (column-names [this] (:cols @builder)) - (metadata [this] (d/datafy (:rsmeta @builder))) + (metadata [this] (d/datafy (.getMetaData rs))) clojure.lang.IPersistentMap (assoc [this k v] @@ -510,7 +517,7 @@ ;; that they can be thrown when the actual functions are called (let [row (try (.getRow rs) (catch Throwable t t)) cols (try (:cols @builder) (catch Throwable t t)) - meta (try (d/datafy (:rsmeta @builder)) (catch Throwable t t))] + meta (try (d/datafy (.getMetaData rs)) (catch Throwable t t))] (with-meta (row-builder @builder) {`core-p/datafy From cfd403301f1919b6865362ec4e3ec1e1451b894e Mon Sep 17 00:00:00 2001 From: Sean Corfield Date: Sun, 7 Jun 2020 09:40:05 -0700 Subject: [PATCH 2/4] Improve readability of informational messages in tests --- test/next/jdbc/datafy_test.clj | 6 ++++-- test/next/jdbc/middleware_test.clj | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/test/next/jdbc/datafy_test.clj b/test/next/jdbc/datafy_test.clj index 0ed73f1..b007bab 100644 --- a/test/next/jdbc/datafy_test.clj +++ b/test/next/jdbc/datafy_test.clj @@ -40,7 +40,8 @@ :schema/exception))) data (set (keys (d/datafy con)))] (when-let [diff (seq (set/difference data reference-keys))] - (println (:dbtype (db)) :connection (sort diff))) + (println (format "%6s :%-10s %s" + (:dbtype (db)) "connection" (str (sort diff))))) (is (= reference-keys (set/intersection reference-keys data))))))) @@ -87,7 +88,8 @@ :rowIdLifetime/exception))) data (set (keys (d/datafy (.getMetaData con))))] (when-let [diff (seq (set/difference data reference-keys))] - (println (:dbtype (db)) :db-meta (sort diff))) + (println (format "%6s :%-10s %s" + (:dbtype (db)) "db-meta" (str (sort diff))))) (is (= reference-keys (set/intersection reference-keys data)))))) (testing "nav to catalogs yields object" diff --git a/test/next/jdbc/middleware_test.clj b/test/next/jdbc/middleware_test.clj index 5c39c41..cdc1319 100644 --- a/test/next/jdbc/middleware_test.clj +++ b/test/next/jdbc/middleware_test.clj @@ -72,4 +72,5 @@ :sql-params-fn start-fn :execute-fn exec-fn}) sql-p) - (println (:dbtype (db)) (:calls @timing) "calls took" (long (:total @timing)) "nanoseconds"))) + (println (format "%6s %d calls took %,10d nanoseconds" + (:dbtype (db)) (:calls @timing) (long (:total @timing)))))) From 509e065fbf8a86c0fdeba510c61a2e6c3ffbeb7e Mon Sep 17 00:00:00 2001 From: Sean Corfield Date: Sun, 7 Jun 2020 10:01:12 -0700 Subject: [PATCH 3/4] Fixes #115 by providing do-commands example --- CHANGELOG.md | 1 + doc/migration-from-clojure-java-jdbc.md | 12 ++++++++++++ test/next/jdbc/test_fixtures.clj | 15 +++++++++++++-- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fd2cca..faed912 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Changes made on master since 1.0.462: * Add tests for `"jtds"` database driver (against MS SQL Server), making it officially supported. * Switch from OpenTable Embedded PostgreSQL to Zonky's version, so that testing can move forward from PostgreSQL 10.11 to 12.2.0. * Address #119 by clarifying realization actions in the docstrings for `row-number`, `column-names`, and `metadata`. +* Address #115 by adding equivalent of `db-do-commands` in the `clojure.java.jdbc` migration guide. * Add log4j2 as a test dependency so that I have better control over logging (which makes debugging easier!). ## Stable Builds diff --git a/doc/migration-from-clojure-java-jdbc.md b/doc/migration-from-clojure-java-jdbc.md index c22f69b..a3b7f6f 100644 --- a/doc/migration-from-clojure-java-jdbc.md +++ b/doc/migration-from-clojure-java-jdbc.md @@ -55,6 +55,18 @@ The `next.jdbc.sql` namespace contains several functions with similarities to `c * `update!` -- similar to `clojure.java.jdbc/update!` but will also accept a hash map of column name/value pairs instead of a partial where clause (vector), * `delete!` -- similar to `clojure.java.jdbc/delete!` but will also accept a hash map of column name/value pairs instead of a partial where clause (vector). +If you were using `db-do-commands` in `clojure.java.jdbc` to execute DDL, the following is the equivalent in `next.jdbc`: + +```clojure +(defn do-commands [connectable commands] + (if (instance? java.sql.Connection connectable) + (with-open [stmt (next.jdbc.prepare/statement connectable)] + (run! #(.addBatch stmt %) commands) + (into [] (.executeBatch stmt))) + (with-open [conn (next.jdbc/get-connection connectable)] + (do-commands conn commands)))) +``` + ### `:identifiers` and `:qualifier` If you are using `:identifiers`, you will need to change to the appropriate `:builder-fn` option with one of `next.jdbc.result-set`'s `as-*` functions. diff --git a/test/next/jdbc/test_fixtures.clj b/test/next/jdbc/test_fixtures.clj index 9b3da86..7d07df9 100644 --- a/test/next/jdbc/test_fixtures.clj +++ b/test/next/jdbc/test_fixtures.clj @@ -4,6 +4,7 @@ "Multi-database testing fixtures." (:require [clojure.string :as str] [next.jdbc :as jdbc] + [next.jdbc.prepare :as prep] [next.jdbc.sql :as sql]) (:import (io.zonky.test.db.postgres.embedded EmbeddedPostgres))) @@ -92,6 +93,16 @@ [] @test-datasource) +(defn- do-commands + "Example from migration docs: this serves as a test for it." + [connectable commands] + (if (instance? java.sql.Connection connectable) + (with-open [stmt (prep/statement connectable)] + (run! #(.addBatch stmt %) commands) + (into [] (.executeBatch stmt))) + (with-open [conn (jdbc/get-connection connectable)] + (do-commands conn commands)))) + (defn with-test-db "Given a test function (or suite), run it in the context of an in-memory H2 database set up with a simple fruit table containing four rows of data. @@ -122,9 +133,9 @@ "AUTO_INCREMENT PRIMARY KEY")] (with-open [con (jdbc/get-connection (ds))] (try - (jdbc/execute-one! con [(str "DROP TABLE " fruit)]) + (do-commands con [(str "DROP TABLE " fruit)]) (catch Exception _)) - (jdbc/execute-one! con [(str " + (do-commands con [(str " CREATE TABLE " fruit " ( ID INTEGER " auto-inc-pk ", NAME VARCHAR(32), From 4045e18f3e9b50695104fd0337b5a513fa3d5687 Mon Sep 17 00:00:00 2001 From: Sean Corfield Date: Sun, 7 Jun 2020 10:02:20 -0700 Subject: [PATCH 4/4] Fix potential reflection warnings from use of next.jdbc.prepare/statement --- CHANGELOG.md | 1 + src/next/jdbc/prepare.clj | 13 +++++++------ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index faed912..d547ad4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ Only accretive/fixative changes will be made from now on. Changes made on master since 1.0.462: * Add tests for `"jtds"` database driver (against MS SQL Server), making it officially supported. * Switch from OpenTable Embedded PostgreSQL to Zonky's version, so that testing can move forward from PostgreSQL 10.11 to 12.2.0. +* Fix potential reflection warnings caused by `next.jdbc.prepare/statement` being incorrectly type-hinted. * Address #119 by clarifying realization actions in the docstrings for `row-number`, `column-names`, and `metadata`. * Address #115 by adding equivalent of `db-do-commands` in the `clojure.java.jdbc` migration guide. * Add log4j2 as a test dependency so that I have better control over logging (which makes debugging easier!). diff --git a/src/next/jdbc/prepare.clj b/src/next/jdbc/prepare.clj index 9a9a06d..7ad757b 100644 --- a/src/next/jdbc/prepare.clj +++ b/src/next/jdbc/prepare.clj @@ -149,12 +149,13 @@ (defn statement "Given a `Connection` and some options, return a `Statement`." - ^java.sql.Statement - ([con] (statement con {})) - ([^Connection con - {:keys [result-type concurrency cursors - fetch-size max-rows timeout] - :as opts}] + (^java.sql.Statement + [con] (statement con {})) + (^java.sql.Statement + [^Connection con + {:keys [result-type concurrency cursors + fetch-size max-rows timeout] + :as opts}] (let [^Statement stmt (cond (and result-type concurrency)