From ecd950d009ba53c027f69affdd68eff60f2ac4e7 Mon Sep 17 00:00:00 2001 From: Sean Corfield Date: Sat, 23 Nov 2024 19:43:54 -0800 Subject: [PATCH] remove experimental name-fn option Signed-off-by: Sean Corfield --- CHANGELOG.md | 3 +- doc/all-the-options.md | 2 - src/next/jdbc/result_set.clj | 5 +- src/next/jdbc/sql/builder.clj | 51 +++++--------- test/next/jdbc/sql/builder_test.clj | 101 +++------------------------- 5 files changed, 32 insertions(+), 130 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c19506a..71fc70c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,12 +6,13 @@ Only accretive/fixative changes will be made from now on. * Address [#288](https://github.com/seancorfield/next-jdbc/issues/288) by adding speculative support for `:dbtype "xtdb"`. * Fix [#287](https://github.com/seancorfield/next-jdbc/issues/287) by merging user-supplied options over `:return-keys true`. * Fix [#282](https://github.com/seancorfield/next-jdbc/issues/282) by tracking raw `Connection` objects for active TXs, which relaxes several of the conditions around nested transactions. + * Removed (experimental) `:name-fn` option since the driver for it no longer exists (qualified columns names in XTDB). * 1.3.955 -- 2024-10-06 * Address [#285](https://github.com/seancorfield/next-jdbc/issues/285) by setting the default Clojure version to the earliest supported (1.10.3) to give a better hint to users. * Update PostgreSQL **Tips & Tricks** example code to fix possible NPE. PR [#284](https://github.com/seancorfield/next-jdbc/pull/284) from [@ExNexu](https://github.com/ExNexu). * Address [#283](https://github.com/seancorfield/next-jdbc/issues/283) by adding a note in the documentation, linking to the PostgreSQL bug report about `ANY(array)`. - * Address [#269](https://github.com/seancorfield/next-jdbc/issues/269) by adding `:name-fn` as an option (primarily for the SQL builder functions, but also for result set processing); the default is `clojure.core/name` but you can now use `next.jdbc.sql.builder/qualified-name` to preserve the qualifier. + * ~Address [#269](https://github.com/seancorfield/next-jdbc/issues/269) by adding `:name-fn` as an option (primarily for the SQL builder functions, but also for result set processing); the default is `clojure.core/name` but you can now use `next.jdbc.sql.builder/qualified-name` to preserve the qualifier.~ _[This was remove in 1.3.next since XTDB no longer supports qualified column names]_ * Update testing deps; `docker-compose` => `docker compose`. * 1.3.939 -- 2024-05-17 diff --git a/doc/all-the-options.md b/doc/all-the-options.md index 5a44239..29400fc 100644 --- a/doc/all-the-options.md +++ b/doc/all-the-options.md @@ -41,7 +41,6 @@ Except for `query` (which is simply an alias for `execute!`), all the "friendly" * `:table-fn` -- the quoting function to be used on the string that identifies the table name, if provided; this also applies to assumed table names when `nav`igating schemas, * `:column-fn` -- the quoting function to be used on any string that identifies a column name, if provided; this also applies to the reducing function context over `plan` and to assumed foreign key column names when `nav`igating schemas. -* `:name-fn` -- may be provided as `next.jdbc.sql.builder/qualified-name` to preserve qualifiers on table and column names; you will need to provide `:table-fn` and/or `:column-fn` as well, in order to quote qualified names properly; new in 1.3.955. They also support a `:suffix` argument which can be used to specify a SQL string that should be appended to the generated SQL string before executing it, e.g., `:suffix "FOR UPDATE"` or, for an `insert!` call `:suffix "RETURNING *"`. The latter is particularly useful for databases, such as SQLite these days, @@ -71,7 +70,6 @@ Any function that might realize a row or a result set will accept: * `: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. 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. * `:column-fn` -- if present, applied to each column name before looking up the column in the `ResultSet` to get that column's value. -* `:name-fn` -- may be provided as `next.jdbc.sql.builder/qualified-name` to preserve qualifiers on keyword used as column names; by default, a keyword like `:foo/bar` is treated as `"bar"` when looking up columns in a `ResultSet`; `:name-fn` allows you to refer to column names that contain `/`, which some databases allow; if both `:name-fn` and `:column-fn` are provided, `:name-fn` is applied first to the keyword (to produce a string) and then `:column-fn` is applied to that; new in 1.3.955. In addition, `execute!` accepts the `:multi-rs true` option to return multiple result sets -- as a vector of result sets. diff --git a/src/next/jdbc/result_set.clj b/src/next/jdbc/result_set.clj index 5525a22..e709cce 100644 --- a/src/next/jdbc/result_set.clj +++ b/src/next/jdbc/result_set.clj @@ -486,10 +486,9 @@ (metadata-preserving) operations on it." [^ResultSet rs opts] (let [builder (delay ((get opts :builder-fn as-maps) rs opts)) - name-fn (:name-fn opts name) name-fn (if (contains? opts :column-fn) - (comp (get opts :column-fn) name-fn) - name-fn)] + (comp (get opts :column-fn) name) + name)] (reify MapifiedResultSet diff --git a/src/next/jdbc/sql/builder.clj b/src/next/jdbc/sql/builder.clj index 94938df..f42c3b2 100644 --- a/src/next/jdbc/sql/builder.clj +++ b/src/next/jdbc/sql/builder.clj @@ -20,20 +20,11 @@ ;; characters in table and column names when building SQL: (def ^:private ^:dynamic *allow-suspicious-entities* false) -(defn qualified-name - "Like `clojure.core/name` but preserves the qualifier, if any. - - Intended for use with `:name-fn`, instead of the default `name`." - [k] - (cond-> (str k) - (keyword? k) - (subs 1))) - (defn- safe-name "A wrapper for `name` that throws an exception if the resulting string looks 'suspicious' as a table or column." - [k name-fn] - (let [entity (name-fn k) + [k] + (let [entity (name k) suspicious #";"] (when-not *allow-suspicious-entities* (when (re-find suspicious entity) @@ -57,18 +48,17 @@ as simple aliases, e.g., `[:foo :bar]`, or as expressions with an alias, e.g., `[\"count(*)\" :total]`." [cols opts] - (let [col-fn (:column-fn opts identity) - name-fn (:name-fn opts name)] + (let [col-fn (:column-fn opts identity)] (str/join ", " (map (fn [raw] (if (vector? raw) (if (keyword? (first raw)) - (str (col-fn (safe-name (first raw) name-fn)) + (str (col-fn (safe-name (first raw))) " AS " - (col-fn (safe-name (second raw) name-fn))) + (col-fn (safe-name (second raw)))) (str (first raw) " AS " - (col-fn (safe-name (second raw) name-fn)))) - (col-fn (safe-name raw name-fn)))) + (col-fn (safe-name (second raw))))) + (col-fn (safe-name raw)))) cols)))) @@ -87,16 +77,15 @@ Applies any `:column-fn` supplied in the options." [key-map clause opts] (let [entity-fn (:column-fn opts identity) - name-fn (:name-fn opts name) [where params] (reduce-kv (fn [[conds params] k v] - (let [e (entity-fn (safe-name k name-fn))] + (let [e (entity-fn (safe-name k))] (if (and (= :where clause) (nil? v)) [(conj conds (str e " IS NULL")) params] [(conj conds (str e " = ?")) (conj params v)]))) [[] []] key-map)] (assert (seq where) "key-map may not be empty") - (into [(str (str/upper-case (safe-name clause name-fn)) " " + (into [(str (str/upper-case (safe-name clause)) " " (str/join (if (= :where clause) " AND " ", ") where))] params))) @@ -111,12 +100,11 @@ `DELETE ...` statement." [table where-params opts] (let [entity-fn (:table-fn opts identity) - name-fn (:name-fn opts name) where-params (if (map? where-params) (by-keys where-params :where opts) (into [(str "WHERE " (first where-params))] (rest where-params)))] - (into [(str "DELETE FROM " (entity-fn (safe-name table name-fn)) + (into [(str "DELETE FROM " (entity-fn (safe-name table)) " " (first where-params) (when-let [suffix (:suffix opts)] (str " " suffix)))] @@ -132,11 +120,10 @@ `INSERT ...` statement." [table key-map opts] (let [entity-fn (:table-fn opts identity) - name-fn (:name-fn opts name) params (as-keys key-map opts) places (as-? key-map opts)] (assert (seq key-map) "key-map may not be empty") - (into [(str "INSERT INTO " (entity-fn (safe-name table name-fn)) + (into [(str "INSERT INTO " (entity-fn (safe-name table)) " (" params ")" " VALUES (" places ")" (when-let [suffix (:suffix opts)] @@ -163,11 +150,10 @@ (assert (seq cols) "cols may not be empty") (assert (seq rows) "rows may not be empty") (let [table-fn (:table-fn opts identity) - name-fn (:name-fn opts name) batch? (:batch opts) params (as-cols cols opts) places (as-? (first rows) opts)] - (into [(str "INSERT INTO " (table-fn (safe-name table name-fn)) + (into [(str "INSERT INTO " (table-fn (safe-name table)) " (" params ")" " VALUES " (if batch? @@ -188,13 +174,12 @@ "Given a column name, or a pair of column name and direction, return the sub-clause for addition to `ORDER BY`." [col opts] - (let [entity-fn (:column-fn opts identity) - name-fn (:name-fn opts name)] + (let [entity-fn (:column-fn opts identity)] (cond (keyword? col) - (entity-fn (safe-name col name-fn)) + (entity-fn (safe-name col)) (and (vector? col) (= 2 (count col)) (keyword? (first col))) - (str (entity-fn (safe-name (first col) name-fn)) + (str (entity-fn (safe-name (first col))) " " (or (get {:asc "ASC" :desc "DESC"} (second col)) (throw (IllegalArgumentException. @@ -231,7 +216,6 @@ `SELECT ...` statement." [table where-params opts] (let [entity-fn (:table-fn opts identity) - name-fn (:name-fn opts name) where-params (cond (map? where-params) (by-keys where-params :where opts) (= :all where-params) @@ -252,7 +236,7 @@ (if-let [cols (seq (:columns opts))] (as-cols cols opts) "*") - " FROM " (entity-fn (safe-name table name-fn)) + " FROM " (entity-fn (safe-name table)) (when-let [clause (first where-params)] (str " " clause)) (when-let [order-by (:order-by opts)] @@ -281,13 +265,12 @@ `UPDATE ...` statement." [table key-map where-params opts] (let [entity-fn (:table-fn opts identity) - name-fn (:name-fn opts name) set-params (by-keys key-map :set opts) where-params (if (map? where-params) (by-keys where-params :where opts) (into [(str "WHERE " (first where-params))] (rest where-params)))] - (-> [(str "UPDATE " (entity-fn (safe-name table name-fn)) + (-> [(str "UPDATE " (entity-fn (safe-name table)) " " (first set-params) " " (first where-params) (when-let [suffix (:suffix opts)] diff --git a/test/next/jdbc/sql/builder_test.clj b/test/next/jdbc/sql/builder_test.clj index 93a5853..38e60d1 100644 --- a/test/next/jdbc/sql/builder_test.clj +++ b/test/next/jdbc/sql/builder_test.clj @@ -13,20 +13,12 @@ (is (= (builder/by-keys {:a nil :b 42 :c "s"} :where {}) ["WHERE a IS NULL AND b = ? AND c = ?" 42 "s"])) (is (= (builder/by-keys {:q/a nil :q/b 42 :q/c "s"} :where {}) - ["WHERE a IS NULL AND b = ? AND c = ?" 42 "s"])) - (is (= (builder/by-keys {:q/a nil :q/b 42 :q/c "s"} :where - {:name-fn builder/qualified-name - :column-fn mysql}) - ["WHERE `q/a` IS NULL AND `q/b` = ? AND `q/c` = ?" 42 "s"]))) + ["WHERE a IS NULL AND b = ? AND c = ?" 42 "s"]))) (testing ":set clause" (is (= (builder/by-keys {:a nil :b 42 :c "s"} :set {}) ["SET a = ?, b = ?, c = ?" nil 42 "s"])) (is (= (builder/by-keys {:q/a nil :q/b 42 :q/c "s"} :set {}) - ["SET a = ?, b = ?, c = ?" nil 42 "s"])) - (is (= (builder/by-keys {:q/a nil :q/b 42 :q/c "s"} :set - {:name-fn builder/qualified-name - :column-fn mysql}) - ["SET `q/a` = ?, `q/b` = ?, `q/c` = ?" nil 42 "s"])))) + ["SET a = ?, b = ?, c = ?" nil 42 "s"])))) (deftest test-as-cols (is (= (builder/as-cols [:a :b :c] {}) @@ -40,34 +32,18 @@ (is (= (builder/as-cols [[:q/a :q/aa] :q/b ["count(*)" :q/c]] {}) "a AS aa, b, count(*) AS c")) (is (= (builder/as-cols [[:q/a :q/aa] :q/b ["count(*)" :q/c]] {:column-fn mysql}) - "`a` AS `aa`, `b`, count(*) AS `c`")) - (is (= (builder/as-cols [:q/a :q/b :q/c] - {:name-fn builder/qualified-name - :column-fn mysql}) - "`q/a`, `q/b`, `q/c`")) - (is (= (builder/as-cols [[:q/a :q/aa] :q/b ["count(*)" :q/c]] - {:name-fn builder/qualified-name - :column-fn mysql}) - "`q/a` AS `q/aa`, `q/b`, count(*) AS `q/c`"))) + "`a` AS `aa`, `b`, count(*) AS `c`"))) (deftest test-as-keys (is (= (builder/as-keys {:a nil :b 42 :c "s"} {}) "a, b, c")) (is (= (builder/as-keys {:q/a nil :q/b 42 :q/c "s"} {}) - "a, b, c")) - (is (= (builder/as-keys {:q/a nil :q/b 42 :q/c "s"} - {:name-fn builder/qualified-name - :column-fn sql-server}) - "[q/a], [q/b], [q/c]"))) + "a, b, c"))) (deftest test-as-? (is (= (builder/as-? {:a nil :b 42 :c "s"} {}) "?, ?, ?")) (is (= (builder/as-? {:q/a nil :q/b 42 :q/c "s"} {}) - "?, ?, ?")) - (is (= (builder/as-? {:q/a nil :q/b 42 :q/c "s"} - {:name-fn builder/qualified-name - :column-fn sql-server}) "?, ?, ?"))) (deftest test-for-query @@ -95,23 +71,7 @@ {:q/id nil} {:table-fn sql-server :column-fn mysql :suffix "FOR UPDATE"}) - ["SELECT * FROM [user] WHERE `id` IS NULL FOR UPDATE"])) - (is (= (builder/for-query - :t/user - {:q/id 9} - {:table-fn sql-server :column-fn mysql :order-by [:x/a [:x/b :desc]] - :name-fn builder/qualified-name}) - ["SELECT * FROM [t/user] WHERE `q/id` = ? ORDER BY `x/a`, `x/b` DESC" 9])) - (is (= (builder/for-query :t/user {:q/id nil} - {:table-fn sql-server :column-fn mysql - :name-fn builder/qualified-name}) - ["SELECT * FROM [t/user] WHERE `q/id` IS NULL"])) - (is (= (builder/for-query :t/user - {:q/id nil} - {:table-fn sql-server :column-fn mysql - :name-fn builder/qualified-name - :suffix "FOR UPDATE"}) - ["SELECT * FROM [t/user] WHERE `q/id` IS NULL FOR UPDATE"]))) + ["SELECT * FROM [user] WHERE `id` IS NULL FOR UPDATE"]))) (testing "by where clause" (is (= (builder/for-query :user @@ -183,13 +143,7 @@ :t/user {:q/opt nil :q/id 9} {:table-fn sql-server :column-fn mysql}) - ["DELETE FROM [user] WHERE `opt` IS NULL AND `id` = ?" 9])) - (is (= (builder/for-delete - :t/user - {:q/opt nil :q/id 9} - {:table-fn sql-server :column-fn mysql - :name-fn builder/qualified-name}) - ["DELETE FROM [t/user] WHERE `q/opt` IS NULL AND `q/id` = ?" 9]))) + ["DELETE FROM [user] WHERE `opt` IS NULL AND `id` = ?" 9]))) (testing "by where clause" (is (= (builder/for-delete :user @@ -200,13 +154,7 @@ :t/user ["id = ? and opt is null" 9] {:table-fn sql-server :column-fn mysql}) - ["DELETE FROM [user] WHERE id = ? and opt is null" 9])) - (is (= (builder/for-delete - :t/user - ["id = ? and opt is null" 9] - {:table-fn sql-server :column-fn mysql - :name-fn builder/qualified-name}) - ["DELETE FROM [t/user] WHERE id = ? and opt is null" 9])))) + ["DELETE FROM [user] WHERE id = ? and opt is null" 9])))) (deftest test-for-update (testing "empty example (would be a SQL error)" @@ -225,13 +173,7 @@ {:q/status 42} {:q/id 9} {:table-fn sql-server :column-fn mysql}) - ["UPDATE [user] SET `status` = ? WHERE `id` = ?" 42 9])) - (is (= (builder/for-update :t/user - {:q/status 42} - {:q/id 9} - {:table-fn sql-server :column-fn mysql - :name-fn builder/qualified-name}) - ["UPDATE [t/user] SET `q/status` = ? WHERE `q/id` = ?" 42 9]))) + ["UPDATE [user] SET `status` = ? WHERE `id` = ?" 42 9]))) (testing "by where clause, with nil set value" (is (= (builder/for-update :user {:status 42, :opt nil} @@ -248,12 +190,7 @@ (is (= (builder/for-insert :t/user {:q/id 9 :q/status 42 :q/opt nil} {:table-fn sql-server :column-fn mysql}) - ["INSERT INTO [user] (`id`, `status`, `opt`) VALUES (?, ?, ?)" 9 42 nil])) - (is (= (builder/for-insert :t/user - {:q/id 9 :q/status 42 :q/opt nil} - {:table-fn sql-server :column-fn mysql - :name-fn builder/qualified-name}) - ["INSERT INTO [t/user] (`q/id`, `q/status`, `q/opt`) VALUES (?, ?, ?)" 9 42 nil]))) + ["INSERT INTO [user] (`id`, `status`, `opt`) VALUES (?, ?, ?)" 9 42 nil]))) (testing "multi-row insert (normal mode)" (is (= (builder/for-insert-multi :user [:id :status] @@ -268,15 +205,7 @@ [35 "world"] [64 "dollars"]] {:table-fn sql-server :column-fn mysql}) - ["INSERT INTO [user] (`id`, `status`) VALUES (?, ?), (?, ?), (?, ?)" 42 "hello" 35 "world" 64 "dollars"])) - (is (= (builder/for-insert-multi :t/user - [:q/id :q/status] - [[42 "hello"] - [35 "world"] - [64 "dollars"]] - {:table-fn sql-server :column-fn mysql - :name-fn builder/qualified-name}) - ["INSERT INTO [t/user] (`q/id`, `q/status`) VALUES (?, ?), (?, ?), (?, ?)" 42 "hello" 35 "world" 64 "dollars"]))) + ["INSERT INTO [user] (`id`, `status`) VALUES (?, ?), (?, ?), (?, ?)" 42 "hello" 35 "world" 64 "dollars"]))) (testing "multi-row insert (batch mode)" (is (= (builder/for-insert-multi :user [:id :status] @@ -291,12 +220,4 @@ [35 "world"] [64 "dollars"]] {:table-fn sql-server :column-fn mysql :batch true}) - ["INSERT INTO [user] (`id`, `status`) VALUES (?, ?)" [42 "hello"] [35 "world"] [64 "dollars"]])) - (is (= (builder/for-insert-multi :t/user - [:q/id :q/status] - [[42 "hello"] - [35 "world"] - [64 "dollars"]] - {:table-fn sql-server :column-fn mysql :batch true - :name-fn builder/qualified-name}) - ["INSERT INTO [t/user] (`q/id`, `q/status`) VALUES (?, ?)" [42 "hello"] [35 "world"] [64 "dollars"]])))) + ["INSERT INTO [user] (`id`, `status`) VALUES (?, ?)" [42 "hello"] [35 "world"] [64 "dollars"]]))))