From a58e5edcf2aed0d3aec772c11876293850411c0c Mon Sep 17 00:00:00 2001 From: Andy Chambers Date: Mon, 13 Aug 2018 17:16:10 +0100 Subject: [PATCH] Consider wrapping union sub-expressions in parenthesis --- README.md | 2 +- src/honeysql/format.cljc | 9 ++++++--- test/honeysql/format_test.cljc | 14 +++++++------- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 449cce4..6de8fa0 100644 --- a/README.md +++ b/README.md @@ -235,7 +235,7 @@ Queries may be united within a :union or :union-all keyword: ```clojure (sql/format {:union [(-> (select :*) (from :foo)) (-> (select :*) (from :bar))]}) -=> ["SELECT * FROM foo UNION SELECT * FROM bar"] +=> ["(SELECT * FROM foo) UNION (SELECT * FROM bar)"] ``` Keywords that begin with `%` are interpreted as SQL function calls: diff --git a/src/honeysql/format.cljc b/src/honeysql/format.cljc index 01a2246..ebbc857 100644 --- a/src/honeysql/format.cljc +++ b/src/honeysql/format.cljc @@ -614,15 +614,18 @@ (defmethod format-clause :union [[_ maps] _] (binding [*subquery?* false] - (string/join " UNION " (map to-sql maps)))) + (string/join " UNION " (map (fn [x] + (str "(" (to-sql x) ")")) maps)))) (defmethod format-clause :union-all [[_ maps] _] (binding [*subquery?* false] - (string/join " UNION ALL " (map to-sql maps)))) + (string/join " UNION ALL " (map (fn [x] + (str "(" (to-sql x) ")")) maps)))) (defmethod format-clause :intersect [[_ maps] _] (binding [*subquery?* false] - (string/join " INTERSECT " (map to-sql maps)))) + (string/join " INTERSECT " (map (fn [x] + (str "(" (to-sql x) ")")) maps)))) (defmethod fn-handler "case" [_ & clauses] (str "CASE " diff --git a/test/honeysql/format_test.cljc b/test/honeysql/format_test.cljc index 38b5a7f..3c3abf0 100644 --- a/test/honeysql/format_test.cljc +++ b/test/honeysql/format_test.cljc @@ -92,17 +92,17 @@ ;; ORDER BY foo ASC (is (= (format {:union [{:select [:foo] :from [:bar1]} {:select [:foo] :from [:bar2]}]}) - ["SELECT foo FROM bar1 UNION SELECT foo FROM bar2"]))) + ["(SELECT foo FROM bar1) UNION (SELECT foo FROM bar2)"]))) (deftest union-all-test (is (= (format {:union-all [{:select [:foo] :from [:bar1]} {:select [:foo] :from [:bar2]}]}) - ["SELECT foo FROM bar1 UNION ALL SELECT foo FROM bar2"]))) + ["(SELECT foo FROM bar1) UNION ALL (SELECT foo FROM bar2)"]))) (deftest intersect-test (is (= (format {:intersect [{:select [:foo] :from [:bar1]} {:select [:foo] :from [:bar2]}]}) - ["SELECT foo FROM bar1 INTERSECT SELECT foo FROM bar2"]))) + ["(SELECT foo FROM bar1) INTERSECT (SELECT foo FROM bar2)"]))) (deftest inner-parts-test (testing "The correct way to apply ORDER BY to various parts of a UNION" @@ -116,7 +116,7 @@ :order-by [[:amount :desc]] :limit 5}]}] :order-by [[:amount :asc]]}) - ["SELECT amount, id, created_on FROM transactions UNION SELECT amount, id, created_on FROM (SELECT amount, id, created_on FROM other_transactions ORDER BY amount DESC LIMIT ?) ORDER BY amount ASC" 5])))) + ["(SELECT amount, id, created_on FROM transactions) UNION (SELECT amount, id, created_on FROM (SELECT amount, id, created_on FROM other_transactions ORDER BY amount DESC LIMIT ?)) ORDER BY amount ASC" 5])))) (deftest compare-expressions-test (testing "Sequences should be fns when in value/comparison spots" @@ -144,7 +144,7 @@ {:select [:foo] :from [:bar2]}] :with [[[:bar {:columns [:spam :eggs]}] {:values [[1 2] [3 4] [5 6]]}]]}) - ["WITH bar (spam, eggs) AS (VALUES (?, ?), (?, ?), (?, ?)) SELECT foo FROM bar1 UNION SELECT foo FROM bar2" 1 2 3 4 5 6]))) + ["WITH bar (spam, eggs) AS (VALUES (?, ?), (?, ?), (?, ?)) (SELECT foo FROM bar1) UNION (SELECT foo FROM bar2)" 1 2 3 4 5 6]))) (deftest union-all-with-cte @@ -152,7 +152,7 @@ {:select [:foo] :from [:bar2]}] :with [[[:bar {:columns [:spam :eggs]}] {:values [[1 2] [3 4] [5 6]]}]]}) - ["WITH bar (spam, eggs) AS (VALUES (?, ?), (?, ?), (?, ?)) SELECT foo FROM bar1 UNION ALL SELECT foo FROM bar2" 1 2 3 4 5 6]))) + ["WITH bar (spam, eggs) AS (VALUES (?, ?), (?, ?), (?, ?)) (SELECT foo FROM bar1) UNION ALL (SELECT foo FROM bar2)" 1 2 3 4 5 6]))) (deftest parameterizer-none (testing "array parameter" @@ -168,7 +168,7 @@ :with [[[:bar {:columns [:spam :eggs]}] {:values [[1 2] [3 4] [5 6]]}]]} :parameterizer :none) - ["WITH bar (spam, eggs) AS (VALUES (1, 2), (3, 4), (5, 6)) SELECT foo FROM bar1 UNION SELECT foo FROM bar2"])))) + ["WITH bar (spam, eggs) AS (VALUES (1, 2), (3, 4), (5, 6)) (SELECT foo FROM bar1) UNION (SELECT foo FROM bar2)"])))) (deftest where-and (testing "should ignore a nil predicate"