Fixes #305 by supporting more complex JOIN source

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')]))))

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]))))))))

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}]})))))
+                              :enabled enabled}]})))))