From e6654f7a226465034d16150d067ac45b5f1c97aa Mon Sep 17 00:00:00 2001 From: Sean Corfield Date: Sat, 6 Aug 2022 21:11:08 -0700 Subject: [PATCH] check for empty where clauses fix #413 --- CHANGELOG.md | 3 +++ doc/options.md | 17 ++++++++++------- src/honey/sql.cljc | 17 ++++++++++++++--- test/honey/sql_test.cljc | 16 +++++++++++++++- 4 files changed, 42 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12c950e..b6e5edb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Changes +* 2.3.next in progress + * Address [#413](https://github.com/seancorfield/honeysql/issues/413) by flagging a lack of `WHERE` clause for `DELETE`, `DELETE FROM`, and `UPDATE` when `:checking :basic` (or `:checking :strict`). + * 2.3.911 -- 2022-07-29 * Address [#418](https://github.com/seancorfield/honeysql/issues/418) by documenting a potential "gotcha" with multi-column `IN` expressions (a change from HoneySQL 1.x). * Fix [#416](https://github.com/seancorfield/honeysql/issues/416) via PR [#417](https://github.com/seancorfield/honeysql/issues/417) from [@corasaurus-hex](https://github.com/corasaurus-hex) -- using the internal default state for the integrity assertion. diff --git a/doc/options.md b/doc/options.md index ff2a80d..d438aa1 100644 --- a/doc/options.md +++ b/doc/options.md @@ -42,15 +42,18 @@ Added in 2.2.858. ## `:checking` -The `:checking` option defaults to `:none`. If `:checking :basic` is -specified, certain obvious errors -- such as `IN` with an empty collection -or `SELECT` with an empty list of columns -- -are treated as an error and an exception is thrown. If `:checking :strict` -is specified, certain dubious constructs -- such as `IN` with a collection -containing `NULL` values -- are also treated as an error and an exception is -thrown. It is expected that this feature will be expanded over time +The `:checking` option defaults to `:none`. +If `:checking :basic` is specified, certain obvious errors +are treated as an error and an exception is thrown. +If `:checking :strict` is specified, certain dubious constructs are also treated as an error and an exception is +thrown. +It is expected that this feature will be expanded over time to help avoid generating illegal SQL. +Here are the checks for each level: +* `:basic` -- `DELETE` and `DELETE FROM` without a `WHERE` clause; `IN` with an empty collection; `SELECT` with an empty list of columns; `UPDATE` without a `WHERE` clause. +* `:strict` -- (all the `:basic` checks plus) `IN` with a collection containing `NULL` values (since this will not match rows). + ## `:dialect` If `:dialect` is provided, `:quoted` will default to `true` for this call. You can still specify `:quoted false` to turn that back off. diff --git a/src/honey/sql.cljc b/src/honey/sql.cljc index 9914650..ea80858 100644 --- a/src/honey/sql.cljc +++ b/src/honey/sql.cljc @@ -926,6 +926,17 @@ (into [(str/join sqls)] params)) [s])) +(defn- check-where + "Given a formatter function, performs a pre-flight check that there is + a non-empty where clause if at least basic checking is enabled." + [formatter] + (fn [k xs] + (when-not (= :none *checking*) + (when-not (seq (:where *dsl*)) + (throw (ex-info (str (sql-kw k) " without a non-empty WHERE clause is dangerous") + {:clause k :where (:where *dsl*)})))) + (formatter k xs))) + (def ^:private base-clause-order "The (base) order for known clauses. Can have items added and removed. @@ -985,9 +996,9 @@ :into #'format-select-into :bulk-collect-into #'format-select-into :insert-into #'format-insert - :update #'format-selector - :delete #'format-selects - :delete-from #'format-selector + :update (check-where #'format-selector) + :delete (check-where #'format-selects) + :delete-from (check-where #'format-selector) :truncate #'format-selector :columns #'format-columns :set #'format-set-exprs diff --git a/test/honey/sql_test.cljc b/test/honey/sql_test.cljc index efefb8b..5e87c3f 100644 --- a/test/honey/sql_test.cljc +++ b/test/honey/sql_test.cljc @@ -796,7 +796,21 @@ ORDER BY id = ? DESC {:checking :strict}))) (is (thrown-with-msg? ExceptionInfo #"does not match" (format {:where [:in :x :?y]} - {:params {:y [nil]} :checking :strict}))))) + {:params {:y [nil]} :checking :strict})))) + (testing "empty WHERE clauses ignored with none" + (is (= ["DELETE FROM foo"] + (format {:delete-from :foo}))) + (is (= ["DELETE foo"] + (format {:delete :foo}))) + (is (= ["UPDATE foo SET x = ?" 1] + (format {:update :foo :set {:x 1}})))) + (testing "empty WHERE clauses flagged in basic mode" + (is (thrown-with-msg? ExceptionInfo #"without a non-empty" + (format {:delete-from :foo} {:checking :basic}))) + (is (thrown-with-msg? ExceptionInfo #"without a non-empty" + (format {:delete :foo} {:checking :basic}))) + (is (thrown-with-msg? ExceptionInfo #"without a non-empty" + (format {:update :foo :set {:x 1}} {:checking :basic}))))) (deftest quoting-:%-syntax (testing "quoting of expressions in functions shouldn't depend on syntax"