From 126ac58c342f804aff39fc0ccff43c710aa20ba3 Mon Sep 17 00:00:00 2001 From: Sean Corfield Date: Wed, 26 Feb 2020 11:48:42 -0800 Subject: [PATCH] Fixes #88 by calling 1-arity keyword Also supports calling `:qualifier-fn` on an empty table name. --- CHANGELOG.md | 1 + doc/all-the-options.md | 2 +- doc/migration-from-clojure-java-jdbc.md | 2 +- src/next/jdbc/result_set.clj | 15 ++++++++------- test/next/jdbc/result_set_test.clj | 18 ++++++++++++++++-- 5 files changed, 27 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ccc729..5604dea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ The following changes have been committed to the **master** branch since the 1.0 * Add PostgreSQL streaming option information to **Tips & Tricks**. * Minor documentation fixes. * Improve `Unknown dbtype` exception message (to clarify that `:classname` is also missing). +* Fix #88 by using 1-arity `keyword` call when table name unavailable (or `:qualifier-fn` returns `nil` or an empty string); also allows `:qualifier-fn` function to be called on empty table name (so `:qualifier-fn (constantly "qual")` will now work much like `clojure.java.jdbc`'s `:qualifier "qual"` worked). * Address #89, #91 by making minor performance tweaks to `next.jdbc.result-set` functions. * Planning to move to MAJOR.MINOR.COMMITS versioning scheme (1.0.375). diff --git a/doc/all-the-options.md b/doc/all-the-options.md index a1cf4c9..df65bf8 100644 --- a/doc/all-the-options.md +++ b/doc/all-the-options.md @@ -45,7 +45,7 @@ Any function that might realize a row or a result set will accept: * `:builder-fn` -- a function that implements the `RowBuilder` and `ResultSetBuilder` protocols; strictly speaking, `plan` and `execute-one!` only need `RowBuilder` to be implemented (and `plan` only needs that if it actually has to realize a row) but most generation functions will implement both for ease of use. * `:label-fn` -- if `:builder-fn` is specified as one of `next.jdbc.result-set`'s `as-modified-*` builders, this option must be present and should specify a string-to-string transformation that will be applied to the column label for each returned column name. -* `:qualifier-fn` -- if `:builder-fn` is specified as one of `next.jdbc.result-set`'s `as-modified-*` builders, this option should specify a string-to-string transformation that will be applied to the table name for each returned column name for which the table name is known. It can be omitted for the `as-unqualified-modified-*` variants. +* `:qualifier-fn` -- if `:builder-fn` is specified as one of `next.jdbc.result-set`'s `as-modified-*` builders, this option should specify a string-to-string transformation that will be applied to the table name for each returned column name. It will be called with an empty string if the table name is not available. It can be omitted for the `as-unqualified-modified-*` variants. > Note: Subject to the caveats above about `:builder-fn`, that means that `plan`, `execute!`, `execute-one!`, and the "friendly" SQL functions will all accept these options for generating rows and result sets. diff --git a/doc/migration-from-clojure-java-jdbc.md b/doc/migration-from-clojure-java-jdbc.md index b66c05d..c22f69b 100644 --- a/doc/migration-from-clojure-java-jdbc.md +++ b/doc/migration-from-clojure-java-jdbc.md @@ -61,7 +61,7 @@ If you are using `:identifiers`, you will need to change to the appropriate `:bu `clojure.java.jdbc`'s default is the equivalent of `as-unqualified-lower-maps` (with the caveat that conflicting column names are not made unique -- see the note above in **Rows and Result Sets**). If you specified `:identifiers identity`, you can use `as-unqualified-maps`. If you provided your own string transformation function, you probably want `as-unqualified-modified-maps` and also pass your transformation function as the `:label-fn` option. -If you used `:qualifier`, you may be able to get the same effect with `as-maps`, `as-lower-maps`, or `as-modified-maps`. Otherwise, you may need to specify the fixed qualifier via the `:label-fn #(str "my_qualifier/" %)`. You might think you could use `:qualifier-fn (constantly "my_qualifier")` for this but it is only called when the column has a known table name so it wouldn't be applied for derived values (and some databases don't provide the table name, so it wouldn't be applied at all for those databases). +If you used `:qualifier`, you can get the same effect with `as-modified-maps` by passing `:qualifier-fn (constantly "my_qualifier")` (and the appropriate `:label-fn` -- either `identity` or `clojure.string/lowercase`). ### `:entities` diff --git a/src/next/jdbc/result_set.clj b/src/next/jdbc/result_set.clj index 296d51b..7fa08be 100644 --- a/src/next/jdbc/result_set.clj +++ b/src/next/jdbc/result_set.clj @@ -29,8 +29,10 @@ "Given `ResultSetMetaData`, return a vector of column names, each qualified by the table from which it came." [^ResultSetMetaData rsmeta opts] - (mapv (fn [^Integer i] (keyword (not-empty (.getTableName rsmeta i)) - (.getColumnLabel rsmeta i))) + (mapv (fn [^Integer i] + (if-let [q (not-empty (.getTableName rsmeta i))] + (keyword q (.getColumnLabel rsmeta i)) + (keyword (.getColumnLabel rsmeta i)))) (range 1 (inc (.getColumnCount rsmeta))))) (defn get-unqualified-column-names @@ -49,11 +51,10 @@ lf (:label-fn opts)] (assert qf ":qualifier-fn is required") (assert lf ":label-fn is required") - (mapv (fn [^Integer i] (keyword (some-> (.getTableName rsmeta i) - (not-empty) - (qf)) - (-> (.getColumnLabel rsmeta i) - (lf)))) + (mapv (fn [^Integer i] + (if-let [q (some-> (.getTableName rsmeta i) (qf) (not-empty))] + (keyword q (-> (.getColumnLabel rsmeta i) (lf))) + (keyword (-> (.getColumnLabel rsmeta i) (lf))))) (range 1 (inc (.getColumnCount rsmeta)))))) (defn get-unqualified-modified-column-names diff --git a/test/next/jdbc/result_set_test.clj b/test/next/jdbc/result_set_test.clj index 7cd1e26..4fe148d 100644 --- a/test/next/jdbc/result_set_test.clj +++ b/test/next/jdbc/result_set_test.clj @@ -134,9 +134,9 @@ (is (map? row)) (is (= 4 (:id row))) (is (= "Orange" (:name row))))) - (testing "custom row builder" + (testing "custom row builder 1" (let [row (p/-execute-one (ds) - ["select * from fruit where id = ?" 3] + ["select fruit.*, id + 100 as newid from fruit where id = ?" 3] (assoc (default-options) :builder-fn rs/as-modified-maps :label-fn str/lower-case @@ -145,7 +145,21 @@ (is (contains? row (column :FRUIT/appearance))) (is (nil? ((column :FRUIT/appearance) row))) (is (= 3 ((column :FRUIT/id) row))) + (is (= 103 (:newid row))) ; no table name here (is (= "Peach" ((column :FRUIT/name) row))))) + (testing "custom row builder 2" + (let [row (p/-execute-one (ds) + ["select fruit.*, id + 100 as newid from fruit where id = ?" 3] + (assoc (default-options) + :builder-fn rs/as-modified-maps + :label-fn str/lower-case + :qualifier-fn (constantly "vegetable")))] + (is (map? row)) + (is (contains? row :vegetable/appearance)) + (is (nil? (:vegetable/appearance row))) + (is (= 3 (:vegetable/id row))) + (is (= 103 (:vegetable/newid row))) ; constant qualifier here + (is (= "Peach" (:vegetable/name row))))) (testing "adapted row builder" (let [row (p/-execute-one (ds) ["select * from fruit where id = ?" 3]