diff --git a/src/honey/sql/helpers.cljc b/src/honey/sql/helpers.cljc index bae9616..b9ba8e8 100644 --- a/src/honey/sql/helpers.cljc +++ b/src/honey/sql/helpers.cljc @@ -11,20 +11,39 @@ (into (vec current) args)) (defn- and-merge + "Recursively merge args into the current expression." [current args] (let [args (remove nil? args)] - (cond (= :and (first current)) - (default-merge current args) - (seq current) - (if (seq args) - (default-merge [:and current] args) - current) - (= 1 (count args)) - (vec (first args)) - (seq args) - (default-merge [:and] args) - :else - (vec current)))) + (cond (= :and (first args)) + (recur current [args]) + (= :or (first args)) + (recur [:or current] (rest args)) + :else + (let [arg (first args) + conj-1 (#{:and :or} (first current)) + conj-2 (#{:and :or} (and (sequential? arg) (first arg)))] + (cond (empty? args) + ;; nothing more to merge: + (vec current) + (and conj-1 conj-2 (= conj-1 conj-2)) + ;; both conjunctions and they match: + (recur (default-merge current (rest arg)) (rest args)) + (and conj-1 conj-2) + ;; both conjunctions but they don't match: + (if (= :and conj-1) + (recur (default-merge current [arg]) (rest args)) + (recur (default-merge [:and current] (rest arg)) (rest args))) + conj-1 + ;; current is conjunction; arg is not + (recur (default-merge (if (= :and conj-1) current [:and current]) [arg]) (rest args)) + (and conj-2 (seq current)) + ;; arg is conjunction; current is not + (recur (default-merge [conj-2 current] (rest arg)) (rest args)) + (seq current) + ;; current non-empty; neither is a conjunction + (recur (default-merge [:and current] [arg]) (rest args)) + :else ; current is empty; use arg as current + (recur (if (sequential? arg) arg [arg]) (rest args))))))) (def ^:private special-merges {:where #'and-merge diff --git a/test/honey/sql/helpers_test.cljc b/test/honey/sql/helpers_test.cljc index 8e5fe6b..a01e9a1 100644 --- a/test/honey/sql/helpers_test.cljc +++ b/test/honey/sql/helpers_test.cljc @@ -483,83 +483,87 @@ ;; these tests are adapted from Cam Saul's PR #283 -#_(deftest merge-where-no-params-test - (doseq [[k [f merge-f]] {"WHERE" [where where] - "HAVING" [having having]}] - (testing "merge-where called with just the map as parameter - see #228" - (let [sqlmap (-> (select :*) - (from :table) - (f [:= :foo :bar]))] - (is (= [(str "SELECT * FROM table " k " foo = bar")] - (sql/format (apply merge-f sqlmap [])))))))) +(deftest merge-where-no-params-test + (doseq [[k [f merge-f]] {"WHERE" [where where] + "HAVING" [having having]}] + (testing "merge-where called with just the map as parameter - see #228" + (let [sqlmap (-> (select :*) + (from :table) + (f [:= :foo :bar]))] + (is (= [(str "SELECT * FROM table " k " foo = bar")] + (sql/format (apply merge-f sqlmap [])))))))) -#_(deftest merge-where-test - (doseq [[k sql-keyword f merge-f] [[:where "WHERE" where where] - [:having "HAVING" having having]]] - (is (= [(str "SELECT * FROM table " sql-keyword " (foo = bar) AND (quuz = xyzzy)")] - (-> (select :*) - (from :table) - (f [:= :foo :bar] [:= :quuz :xyzzy]) - sql/format))) - (is (= [(str "SELECT * FROM table " sql-keyword " (foo = bar) AND (quuz = xyzzy)")] - (-> (select :*) - (from :table) - (f [:= :foo :bar]) - (merge-f [:= :quuz :xyzzy]) - sql/format))) - (testing "Should work when first arg isn't a map" - (is (= {k [:and [:x] [:y]]} - (merge-f [:x] [:y])))) - (testing "Shouldn't use conjunction if there is only one clause in the result" - (is (= {k [:x]} - (merge-f {} [:x])))) - (testing "Should be able to specify the conjunction type" - (is (= {k [:or [:x] [:y]]} - (merge-f {} - :or - [:x] [:y])))) - (testing "Should ignore nil clauses" - (is (= {k [:or [:x] [:y]]} - (merge-f {} - :or - [:x] nil [:y])))))) - -#_(deftest merge-where-combine-clauses-test - (doseq [[k f] {:where where - :having having}] - (testing (str "Combine new " k " clauses into the existing clause when appropriate. (#282)") - (testing "No existing clause" - (is (= {k [:and [:x] [:y]]} - (f {} - [:x] [:y])))) - (testing "Existing clause is not a conjunction." - (is (= {k [:and [:a] [:x] [:y]]} - (f {k [:a]} - [:x] [:y])))) - (testing "Existing clause IS a conjunction." - (testing "New clause(s) are not conjunctions" - (is (= {k [:and [:a] [:b] [:x] [:y]]} - (f {k [:and [:a] [:b]]} - [:x] [:y])))) - (testing "New clauses(s) ARE conjunction(s)" - (is (= {k [:and [:a] [:b] [:x] [:y]]} - (f {k [:and [:a] [:b]]} - [:and [:x] [:y]]))) - (is (= {k [:and [:a] [:b] [:x] [:y]]} - (f {k [:and [:a] [:b]]} - [:and [:x]] - [:y]))) - (is (= {k [:and [:a] [:b] [:x] [:y]]} - (f {k [:and [:a] [:b]]} - [:and [:x]] - [:and [:y]]))))) - (testing "if existing clause isn't the same conjunction, don't merge into it" - (testing "existing conjunction is `:or`" - (is (= {k [:and [:or [:a] [:b]] [:x] [:y]]} - (f {k [:or [:a] [:b]]} - [:x] [:y])))) - (testing "pass conjunction type as a param (override default of :and)" - (is (= {k [:or [:and [:a] [:b]] [:x] [:y]]} - (f {k [:and [:a] [:b]]} +(deftest merge-where-test + (doseq [[k sql-keyword f merge-f] [[:where "WHERE" where where] + [:having "HAVING" having having]]] + (is (= [(str "SELECT * FROM table " sql-keyword " (foo = bar) AND (quuz = xyzzy)")] + (-> (select :*) + (from :table) + (f [:= :foo :bar] [:= :quuz :xyzzy]) + sql/format))) + (is (= [(str "SELECT * FROM table " sql-keyword " (foo = bar) AND (quuz = xyzzy)")] + (-> (select :*) + (from :table) + (f [:= :foo :bar]) + (merge-f [:= :quuz :xyzzy]) + sql/format))) + (testing "Should work when first arg isn't a map" + (is (= {k [:and [:x] [:y]]} + (merge-f [:x] [:y])))) + (testing "Shouldn't use conjunction if there is only one clause in the result" + (is (= {k [:x]} + (merge-f {} [:x])))) + (testing "Should be able to specify the conjunction type" + (is (= {k [:or [:x] [:y]]} + (merge-f {} :or - [:x] [:y])))))))) + [:x] [:y])))) + (testing "Should ignore nil clauses" + (is (= {k [:or [:x] [:y]]} + (merge-f {} + :or + [:x] nil [:y])))))) + +(deftest merge-where-combine-clauses-test + (doseq [[k f] {:where where + :having having}] + (testing (str "Combine new " k " clauses into the existing clause when appropriate. (#282)") + (testing "No existing clause" + (is (= {k [:and [:x] [:y]]} + (f {} + [:x] [:y])))) + (testing "Existing clause is not a conjunction." + (is (= {k [:and [:a] [:x] [:y]]} + (f {k [:a]} + [:x] [:y])))) + (testing "Existing clause IS a conjunction." + (testing "New clause(s) are not conjunctions" + (is (= {k [:and [:a] [:b] [:x] [:y]]} + (f {k [:and [:a] [:b]]} + [:x] [:y])))) + (testing "New clauses(s) ARE conjunction(s)" + (is (= {k [:and [:a] [:b] [:x] [:y]]} + (f {k [:and [:a] [:b]]} + [:and [:x] [:y]]))) + (is (= {k [:and [:a] [:b] [:x] [:y]]} + (f {k [:and [:a] [:b]]} + [:and [:x]] + [:y]))) + (is (= {k [:and [:a] [:b] [:x] [:y]]} + (f {k [:and [:a] [:b]]} + [:and [:x]] + [:and [:y]]))))) + (testing "if existing clause isn't the same conjunction, don't merge into it" + (testing "existing conjunction is `:or`" + (is (= {k [:and [:or [:a] [:b]] [:x] [:y]]} + (f {k [:or [:a] [:b]]} + [:x] [:y])))) + (testing "pass conjunction type as a param (override default of :and)" + (is (= {k [:or [:and [:a] [:b]] [:x] [:y]]} + (f {k [:and [:a] [:b]]} + :or + [:x] [:y])))))))) + +(comment + (where {:where [:and [:a] [:b]]} [:and [:x] [:y]]) + .)