A few small changes/fixes to join behaviour, and code cleanup

Fix compilation of cross joins.

Remove the sort clauses on queries when they become subqueries in joins because
then the sorting means nothing (although when take/drop stuff is added the sort
will be relevant, so we'll see about that).

Favour the left side of a join, which is mostly only relevant for the outer
join case (and technically it's not quite right for full-outer joins, maybe,
but I'll get to that later).

4 files changed, 49 insertions, 155 deletions

diff --git a/src/clojure_sql/compiler.clj b/src/clojure_sql/compiler.clj
index 60a5530..9ba0c3a 100644
--- a/src/clojure_sql/compiler.clj
+++ b/src/clojure_sql/compiler.clj
@@ -2,7 +2,7 @@
   (:refer-clojure :exclude [compile sequence])
   (:require [clojure.string :as string]
             [clojure-sql.query :refer [query?]]
-            [clojure-sql.util :as u]
+            [clojure-sql.util :as u :refer [named?]]
             [clojure-sql.writer :as w :refer [return lift p-lift sequence do-m tell >>]]))
 
 (defn add-parentheses [s]
@@ -105,9 +105,9 @@
   (if (or (= table alias) (nil? alias))
     (return (table-name db table))
     ($str (condp #(%1 %2) table
-                 query? ($add-parentheses (compile-query db table))
-                 named? (return (table-name db table))
-                 (compile-expression db table))
+            query? ($add-parentheses (compile-query db table))
+            named? (return (table-name db table))
+            (compile-expression db table))
           (return " AS ")
           (return (table-name db alias)))))
 
@@ -191,17 +191,22 @@
     (return nil)))
 
 (defmulti compile-query (fn [db _] db))
-(defmethod compile-query :default [db {:keys [tables fields joins where sort group having]}]
-  ($str (return "SELECT ")
-        (compile-fields db fields)
-        (if tables
-          ($str (return " FROM ")
-                (compile-tables db joins tables))
-          ($str ""))
-        (compile-where db where)
-        (compile-group db group)
-        (compile-having db having)
-        (compile-sort db sort)))
+(defmethod compile-query :default [db {:keys [tables fields joins where sort group having union]}]
+  (if union
+    (->> union
+         (map (partial compile-query db))
+         (apply sequence)
+         ((p-lift string/join " UNION ")))
+    ($str (return "SELECT ")
+          (compile-fields db fields)
+          (if tables
+            ($str (return " FROM ")
+                  (compile-tables db joins tables))
+            ($str ""))
+          (compile-where db where)
+          (compile-group db group)
+          (compile-having db having)
+          (compile-sort db sort))))
 
 
 
@@ -241,7 +246,6 @@
 (defn insert! [db query & records]
   {:pre [(empty? (:joins query))]}
   ;; some code here
-  #_($str (return "Carlo"))
   )
 
 (defn update! [db query & partial-records]
diff --git a/src/clojure_sql/core.clj b/src/clojure_sql/core.clj
index 4e3db34..72438a6 100644
--- a/src/clojure_sql/core.clj
+++ b/src/clojure_sql/core.clj
@@ -1,5 +1,5 @@
 (ns clojure-sql.core
-  (:refer-clojure :exclude [sort-by])
+  (:refer-clojure :exclude [sort])
   (:require [clojure.set :as set]
             [clojure-sql.compiler :as c]
             [clojure-sql.dsl :as d]
@@ -17,85 +17,12 @@
   (binding [*out* writer]
     (pr (c/compile nil query))))
 
-(comment
 
-  (def table #'d/table)
-  (def project #'d/project)
-  (def rename #'d/rename)
-  (def join #'d/join)
-  (def select #'d/select)
-  (def sort-by #'d/sort-by)
-
-  (def insert! #'c/insert!)
-  (def update! #'c/update!)
-  (def delete! #'c/delete!))
-
-
-
-
-
-
-(comment
-
-  (let [users (-> (table :users)
-                  (project [:id :username :password])
-                  (select '(= :deleted false)))
-        people (-> (table :people)
-                   (project [:id :fname :sname])
-                   (select '(= :deleted false)))
-        uid-pid-match '(= :uid :pid)
-        is-carlo `(= :fname "Carlo'; SELECT * FROM users --")]
-    (-> (join (-> users
-                  (rename {:id :uid}))
-              (join (-> people
-                        (rename {:id :pid}))
-                    (-> (table {:others :o})
-                        (project {:id :oid}))
-                    :on '(= :pid :oid))
-              :on uid-pid-match)
-        (select is-carlo)
-        (project [:fname :sname :oid])))
-
-  (-> (table :users)
-      (project [:username])
-      (join (table :something-else-with-a-username))
-      (select '(or (= :username "john")
-                   (not (= :username "carlo"))))
-      (project [:username]))
-
-  (-> (table {:nodes :child})
-      (project [:parent-id, :name])
-      (rename {:name :child.name
-               :parent-id :pid})
-      (join (-> (table {:nodes :parent})
-                (project [:id, :name])
-                (rename {:name :parent.name}))
-            :on '(= :pid :id))
-      (project [:child.name :parent.name]))
-
-  (-> (table :users)
-      (project [:id])
-      (join (-> (table :people)
-                (project [:id]))))
-  (-> (table :users)
-      (project [:id :name])
-      (rename {:id :name
-               :name :id}))
-  (-> (table :users)
-      (project {:id :name
-                :name :id})
-      (select '(= :id 10)))
-
-  (-> (table :anotherStack)
-      (project [:anotherNumber])
-      (join (-> (table :collection)
-                (project [:number]))))
-
-  (-> (table :anotherStack)
-      (project [:anotherNumber])
-      (join (-> (table :collection)
-                (project [:number])))
-      (select '(is-okay 10)))
-
-  (-> (table :users)
-      (select '(= (left :username 1) "bloo"))))
+(def table #'d/table)
+(def project #'d/project)
+(def rename #'d/rename)
+(def join #'d/join)
+(def select #'d/select)
+(def group #'d/group)
+(def having #'d/having)
+(def sort #'d/sort)
diff --git a/src/clojure_sql/dsl.clj b/src/clojure_sql/dsl.clj
index a31a913..c091d0e 100644
--- a/src/clojure_sql/dsl.clj
+++ b/src/clojure_sql/dsl.clj
@@ -35,7 +35,7 @@
                    :query query})))
 
 (defn ^:private same-rename-error [alias renames & [query]]
-  (throw (ex-info (str "Cannot rename two fields to the same alias: " alias)
+  (throw (ex-info (str "Cannot rename multiple fields to the same alias: " alias)
                   {:alias alias
                    :renames renames
                    :query query})))
@@ -70,13 +70,16 @@
                    :right right
                    :type type })))
 
+(defn ^:private invalid-union [queries]
+  (throw (ex-info "Cannot union queries with different fields"
+                  {:queries queries})))
+
 (defn table [arg]
-  (into (q/->Query)
-        (let [alias (keyword (gensym (if (u/named? arg)
-                                       (name arg)
-                                       "table")))]
-          {:tables {alias arg}
-           :joins [alias]})))
+  (q/map->Query (let [alias (keyword (gensym (if (u/named? arg)
+                                               (name arg)
+                                               "table")))]
+                  {:tables {alias arg}
+                   :joins [alias]})))
 
 (defn ^:private rename-table [query from to]
   (q/map->Query (walk/prewalk-replace {from to} (into {} query))))
@@ -98,7 +101,7 @@
   (let [table (if (= (count (:tables query)) 1)
                 (-> query :tables first key))
         alias-lookup (or (:fields query) {})
-        get-real-name #(resolve-fields table alias-lookup %)
+        get-real-name #(process-expression table alias-lookup %)
         fields (if (map? fields)
                  fields
                  (zipmap fields fields))]
@@ -120,10 +123,6 @@
                        {} fields))))
 
 (defn ^:private rename-with-map [query field-renames]
-  {:pre [;; the intersection of the new aliases with the old aliases NOT renamed by this operation
-         #_(empty? (set/intersection (set (vals field-renames))
-                                   (set/difference (set (keys (:fields query)))
-                                                   (set (keys field-renames)))))]}
   (doseq [[field alias] field-renames]
     (if-not (contains? (:fields query) field)
       (missing-rename-error field field-renames query)))
@@ -161,10 +160,14 @@
 (defn ^:private joinable? [query] 
   (and (nil? (:group query))
        (nil? (:having query))))
+
 (defn ^:private convert-to-subquery [query]
   (-> (table query)
       (project (keys (:fields query)))))
 
+(defn ^:private remove-sort [query]
+  (dissoc query :sort))
+
 (def ^:private valid-join-type? (comp boolean #{:cross :inner :outer :full-outer}))
 (defn join [left right & {:keys [on type]}]
   (if (= type :right)
@@ -174,10 +177,10 @@
                  type)
           left (if (joinable? left)
                  left
-                 (convert-to-subquery left))
+                 (convert-to-subquery (remove-sort left)))
           right (if (joinable? right)
                   right
-                  (convert-to-subquery right))
+                  (convert-to-subquery (remove-sort right)))
           common-tables (set/intersection (set (keys (:tables left)))
                                           (set (keys (:tables right))))
           right (reduce (fn [query [alias table]]
@@ -187,7 +190,7 @@
           merged-tables (merge (:tables left) (:tables right))
           common-fields (set/intersection (set (keys (:fields left)))
                                           (set (keys (:fields right))))
-          merged-fields (merge (:fields left) (:fields right))
+          merged-fields (merge (:fields right) (:fields left)) ;; favour the left name for outer joins
           join-condition (cond
                           (nil? on) (let [implicit (map (fn [field]
                                                           `(~'=
@@ -260,43 +263,3 @@
         resolved-expression (process-expression table-name (:fields query) expression)
         new-having (combine-conjunctions old-having resolved-expression)]
     (assoc query :having new-having)))
-
-(-> (table :x)
-    (project [:x])
-    (join (-> (table :y)
-              (select `(= :id 10)))
-          :type :right))
-
-(let [users (-> (table :users)
-                (project [:id]))]
-  (join users users))
-
-(-> (table :x)
-    (project [:x])
-    (join (-> (table :y)
-              #_(select `(= :id 10)))
-          :type :full-outer))
-
-(-> (table :x)
-    (project [:x])
-    (join (-> (table :y)
-              (project [:x]))))
-
-#_(-> (table :x)
-    (project [:x])
-    (rename {:y :z}))
-
-(-> (table :x)
-    (project [:x])
-    (rename {:x :y}))
-
-(-> (table :people)
-    (project '{(count *) :count})
-    (group [:age])
-    (sort [:age])
-    (join (-> (table :number-count)
-              (project [:number :text])
-              (sort [:number]))
-          :on `(= :count :number))
-    (project [:text])
-    println)
diff --git a/src/clojure_sql/query.clj b/src/clojure_sql/query.clj
index 00eb212..218ae7a 100644
--- a/src/clojure_sql/query.clj
+++ b/src/clojure_sql/query.clj
@@ -5,9 +5,9 @@
   (alter-var-root #'*query-deref-behaviour* (constantly f))
   nil)
 
-(defrecord ^:private Query [] 
+(defrecord ^:private Query []
   clojure.lang.IDeref
   (deref [this] (*query-deref-behaviour* this)))
 
-(def query? (partial instance? (class (->Query))))
+(def query? (partial instance? Query))