diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d55dd3..aa29811 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ * 2.0.next in progress * The documentation continues to be expanded and clarified in response to feedback! + * Fix #322 by rewriting/simplifying `WHERE`/`HAVING` merge logic. * Fix #310 by adding support for `FILTER`, `WITHIN GROUP`, and `ORDER BY` (as an expression), from [nilenso/honeysql-postgres](https://github.com/nilenso/honeysql-postgres) 0.4.112. These are [Special Syntax](doc/special-syntax.md) and there are also helpers for `filter` and `within-group` -- so **be careful about referring in all of `honey.sql.helpers`** since it will now shadow `clojure.core/filter` (it already shadows `for`, `group-by`, `into`, `partition-by`, `set`, and `update`). * Fix #308 by supporting join clauses in `join-by` (and correcting the helper docstring). diff --git a/src/honey/sql/helpers.cljc b/src/honey/sql/helpers.cljc index 230d6b9..27f2ca7 100644 --- a/src/honey/sql/helpers.cljc +++ b/src/honey/sql/helpers.cljc @@ -11,53 +11,67 @@ (defn- default-merge [current args] (c/into (vec current) args)) -(defn- and-merge - "Merge a single conjunction expression into an existing one. - This merges `AND` to avoid nesting." - [current arg] - (if-let [conj' (and (sequential? arg) - (ident? (first arg)) - (#{:and :or} (keyword (first arg))))] - (cond (= conj' (first current)) - (c/into (vec current) (rest arg)) - (seq current) - (c/into [conj' current] (rest arg)) - :else - (c/into [conj'] (rest arg))) - (cond (#{:and 'and} (first current)) - (conj (vec current) arg) - (seq current) - (conj [:and current] arg) - :else - (conj [:and] arg)))) +(defn- sym->kw + "Given a symbol, produce a keyword, retaining the namespace + qualifier, if any." + [s] + (if (symbol? s) + (if-let [n (namespace s)] + (keyword n (name s)) + (keyword (name s))) + s)) -(defn- and-merges - "Merge multiple conjunction expressions into an existing, - possibly empty, expression. This ensures AND expressions - are merged and that we do not end up with a single AND - or OR expression." +(defn- conjunction? + [e] + (and (ident? e) + (contains? #{:and :or} (sym->kw e)))) + +(defn- simplify-logic + "For Boolean expressions, simplify the logic to make + the output expression less nested. Finding :and or + :or with a single condition can be lifted. Finding + a conjunction inside the same conjunction can be + merged. + Always called on an expression that begins with a conjunction!" + [e] + (if (= 1 (count (rest e))) + (fnext e) + (let [conjunction (sym->kw (first e))] + (reduce (fn [acc e] + (if (and (sequential? e) + (conjunction? (first e)) + (= conjunction (sym->kw (first e)))) + (c/into acc (rest e)) + (conj acc e))) + [conjunction] + (rest e))))) + +(defn- conjunction-merge + "Merge for where/having. We ignore nil expressions. + By default, we combine with AND unless the new expression + begins with a conjunction, in which case use that to + combine the new expression. Then we perform some + simplifications to reduce nesting." [current args] (let [args (remove nil? args) - result - (cond (ident? (first args)) - (and-merges current [args]) - (seq args) - (let [[arg & args] args] - (and-merges (and-merge current arg) args)) + [conjunction args] + (cond (conjunction? (first args)) + [(first args) (rest args)] + (ident? (first args)) + [:and [args]] :else - current)] - (case (count result) - 0 nil - 1 (if (sequential? (first result))(first result) result) - 2 (if (#{:and :or} (first result)) - (second result) - result) - result))) + [:and args])] + (if (seq args) + (-> [conjunction] + (cond-> (seq current) (conj current)) + (c/into args) + (simplify-logic)) + current))) (def ^:private special-merges "Identify the conjunction merge clauses." - {:where #'and-merges - :having #'and-merges}) + {:where #'conjunction-merge + :having #'conjunction-merge}) (defn- helper-merge [data k args] (if-let [merge-fn (special-merges k)] diff --git a/test/honey/sql/helpers_test.cljc b/test/honey/sql/helpers_test.cljc index 14d1ce3..355bf71 100644 --- a/test/honey/sql/helpers_test.cljc +++ b/test/honey/sql/helpers_test.cljc @@ -843,3 +843,17 @@ " SUM(q) FILTER (WHERE x IS NULL) AS b," " FOO(y) WITHIN GROUP (ORDER BY x ASC)") 5 10])))) + +(deftest issue-322 + (testing "Combining WHERE clauses with conditions" + (is (= {:where [:and [:= :a 1] [:or [:= :b 2] [:= :c 3]]]} + (where [:= :a 1] [:or [:= :b 2] [:= :c 3]]))) + (is (= (-> (where :or [:= :b 2] [:= :c 3]) ; or first + (where := :a 1)) ; then implicit and + (-> (where := :b 2) ; implicit and + (where :or [:= :c 3]) ; then explicit or + (where := :a 1)))) ; then implicit and + (is (= {:where [:and [:or [:= :b 2] [:= :c 3]] [:= :a 1]]} + (where [:or [:= :b 2] [:= :c 3]] [:= :a 1]) + (-> (where :or [:= :b 2] [:= :c 3]) ; explicit or + (where := :a 1)))))) ; then implicit and