fix #505 by rewriting helper-merge

CHANGELOG.md

@@ -1,6 +1,7 @@
 # Changes
 
 * 2.4.next in progress
+  * Fix [#505](https://github.com/seancorfield/honeysql/issues/505) by rewriting the helper merge function to handle both keywords and symbols properly.
   * Address [#503](https://github.com/seancorfield/honeysql/issues/503) by adding `:at-time-zone` special syntax.
   * Address [#504](https://github.com/seancorfield/honeysql/issues/504) for BigQuery support, by adding special syntax for ignore/respect nulls, as well as new `:distinct` and `:expr` clauses to allow expressions to be qualified with SQL clauses. The latter will probably be useful for other dialects too.
 

src/honey/sql/helpers.cljc

@@ -71,6 +71,16 @@
       (keyword (name s)))
     s))
 
+(defn- kw->sym
+  "Given a keyword, produce a symbol, retaining the namespace
+  qualifier, if any."
+  [k]
+  (if (keyword? k)
+    (if-let [n (namespace k)]
+      (symbol n (name k))
+      (symbol (name k)))
+    k))
+
 (defn- conjunction?
   [e]
   (and (ident? e)
@@ -134,11 +144,26 @@
    :having             #'conjunction-merge})
 
 (defn- helper-merge [data k args]
-  (if-let [merge-fn (special-merges k)]
+  (let [k'  (sym->kw k)
-    (if-some [clause (merge-fn (get data k) args)]
+        k   (kw->sym k)
-      (assoc data k clause)
+        d   (get data k)
-      data)
+        d'  (get data k')
-    (clojure.core/update data k default-merge args)))
+        mf  (special-merges k')
+        mf' (or mf default-merge)]
+    (cond (some? d)
+          (if-some [clause (mf' d args)]
+            (assoc data k clause)
+            data)
+          (some? d')
+          (if-some [clause (mf' d' args)]
+            (assoc data k' clause)
+            data)
+          mf
+          (if-some [clause (mf nil args)]
+            (assoc data k' clause)
+            data)
+          :else
+          (clojure.core/update data k' default-merge args))))
 
 (defn- generic [k args]
   (if (map? (first args))

test/honey/sql/helpers_test.cljc

@@ -913,3 +913,24 @@
            (where false)))
     (is (= ["SELECT * FROM table WHERE FALSE"]
            (sql/format {:select [:*] :from [:table] :where false})))))
+
+(deftest issue-505
+  (testing "where should merge symbols/keywords correctly"
+    (is (= '{where [:and (= a 1) [:= :b 2]]}
+           (-> '{where (= a 1)}
+               (where [:= :b 2]))))
+    (is (= '{where (= a 1)}
+           (-> '{where (= a 1)}
+               (where))))
+    (is (= '{:where [:and (= a 1) [:= :b 2]]}
+           (-> '{:where (= a 1)}
+               (where [:= :b 2]))))
+    (is (= '{:where (= a 1)}
+           (-> '{:where (= a 1)}
+               (where))))
+    (is (= '{:where [:= :b 2]}
+           (-> '{}
+               (where [:= :b 2]))))
+    (is (= '{}
+           (-> '{}
+               (where))))))