diff --git a/src/honey/sql.cljc b/src/honey/sql.cljc index f45d527..6e50bec 100644 --- a/src/honey/sql.cljc +++ b/src/honey/sql.cljc @@ -372,16 +372,19 @@ (defn- format-join [k clauses] (let [[sqls params] (reduce (fn [[sqls params] [j e]] - (let [sqls (conj sqls - (sql-kw (if (= :join k) :inner-join k)) - (format-entity-alias j))] + (let [[sql-j & params-j] + (format-selects-common + (sql-kw (if (= :join k) :inner-join k)) + true + [j]) + sqls (conj sqls sql-j)] (if (and (sequential? e) (= :using (first e))) [(conj sqls "USING" (str "(" (str/join ", " (map #'format-entity-alias (rest e))) ")")) - params] + (into params params-j)] (let [[sql & params'] (when e (format-expr e))] [(cond-> sqls e (conj "ON" sql)) (into params params')])))) diff --git a/test/honey/sql/helpers_test.cljc b/test/honey/sql/helpers_test.cljc index 35274e7..320ca19 100644 --- a/test/honey/sql/helpers_test.cljc +++ b/test/honey/sql/helpers_test.cljc @@ -480,3 +480,86 @@ ;; three arguments with an alias and columns: (is (= (sql/format (insert-into '(transport t) '(id, name) '{select (*) from (cars)})) ["INSERT INTO transport AS t (id, name) SELECT * FROM cars"]))) + +;; 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-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]]} + :or + [:x] [:y])))))))) diff --git a/test/honey/sql_test.cljc b/test/honey/sql_test.cljc index 0868889..3a475b6 100644 --- a/test/honey/sql_test.cljc +++ b/test/honey/sql_test.cljc @@ -585,7 +585,6 @@ ORDER BY id = ? DESC (h/order-by [[:= :id 123] :desc])) {:pretty true})))) - (deftest issue-299-test (let [name "test field" ;; this was a bug in v1 -- adding here to prevent regression: @@ -593,4 +592,4 @@ ORDER BY id = ? DESC (is (= ["INSERT INTO table (name, enabled) VALUES (?, (TRUE, ?))" name (second enabled)] (format {:insert-into :table :values [{:name name - :enabled enabled}]}))))) \ No newline at end of file + :enabled enabled}]})))))