Remove the `having` function, incorporate into `select`

The `having` function duplicated the intended functionality of `select`, but in
a bad way. It only applied to a restricted case, and its existence meant that
`select` provided an escape-hatch with which we could cause some unexpected
behaviour. By consolidating the two functions into `select` we remove the
escape hatch as well as simplify the model.

Selection on grouped queries may now introduce a new subquery, but only in
situations where the behaviour is unpredictable (ie. selecting on non-grouped
attributes).

1 files changed, 22 insertions, 19 deletions

diff --git a/src/clojure_sql/dsl.clj b/src/clojure_sql/dsl.clj
index e33bbfb..8e4a24d 100644
--- a/src/clojure_sql/dsl.clj
+++ b/src/clojure_sql/dsl.clj
@@ -268,6 +268,14 @@
   quote can be used to suppress evaluation of terms in the
   expression.
 
+  If the query being filtered is the result of a previous grouping
+  operation then a `select` operation will compile in one of two
+  different ways:
+    1. If the selection is entirely made on grouped attributes it will
+    compile into a `having` clause in the resulting query.
+    2. If the selection includes non-grouped attributes then a
+    subquery will be introduced.
+
   Example:
     (select query `(= :id 10)) - filter for an id of 10
     (select query {:id 10}) - equivalent to the above
@@ -275,9 +283,20 @@
   [query expression]
   (let [expression (if (map? expression)
                      (reduce combine-conjunctions
-                             (map #(cons '= %) expression)))
-        resolved-expression (process-expression (:tables query) (:fields query) expression)]
-    (update-in query [:where] combine-conjunctions resolved-expression)))
+                             (map #(cons '= %) expression))
+                     expression)
+        expression-fields (filter keyword? (flatten expression))
+        bad-expression-fields (set/difference (set (process-expression nil
+                                                                       (:fields query)
+                                                                       expression-fields))
+                                              (set (:group query)))
+        where? (nil? (:group query))
+        having? (empty? bad-expression-fields)]
+    (if (or where? having?)
+      (let [attr (if where? :where :having) 
+            resolved-expression (process-expression (:tables query) (:fields query) expression)]
+        (update-in query [attr] combine-conjunctions resolved-expression))
+      (recur (convert-to-subquery query) expression))))
 
 (defn sort
   "Apply a sort to a query. Replaces any existing sort on the
@@ -320,22 +339,6 @@
     (assoc query :group (map (partial resolve-field (:tables query) (:fields query))
                              fields-seq))))
 
-(defn having
-  "Apply a filter to the groupings of a query.
-
-  `expression` is the same as a `select`, but may only reference
-  fields by which the query is grouped (see `group`) or other fields
-  within aggregating functions (eg. count).
-
-  Example:
-    (having query `(< (sum :age) 100)) - select groups with a combined
-  age under 100"
-  [query expression]
-  (let [old-having (:having query)
-        resolved-expression (process-expression (:tables query) (:fields query) expression)
-        new-having (combine-conjunctions old-having resolved-expression)]
-    (assoc query :having new-having)))
-
 (defn take
   "Limit the number of results of a query.