convert order_by

src/metabase/driver/generic_sql/query_processor.clj

@@ -170,27 +170,21 @@
 (defmethod apply-form :limit [[_ value]]
   `(limit ~value))
 
-;; TODO
-(defmethod apply-form :order_by [[_ order-by-pairs]]
-  (when-not (empty? order-by-pairs)
-    (->> order-by-pairs
-         (map (fn [pair] (when-not (vector? pair) (throw (Exception. "order_by clause must consists of pairs like [field_id \"ascending\"]"))) pair))
-         (mapv (fn [[field asc-desc]]
-                 {:pre [(string? asc-desc)]}
-                 `(order ~(match [field]
-                            [field-id :guard integer?] (field-id->kw field-id)
-                            [["aggregation" 0]]        (let [[ag] (:aggregation (:query qp/*query*))]
-                                                         `(raw ~(case ag
-                                                                  "avg"      "\"avg\""   ; based on the type of the aggregation
-                                                                  "count"    "\"count\"" ; make sure we ask the DB to order by the
-                                                                  "distinct" "\"count\"" ; name of the aggregate field
-                                                                  "stddev"   "\"stddev\""
-                                                                  "sum"      "\"sum\""))))
-                         ~(case asc-desc
-                            "ascending" :ASC
-                            "descending" :DESC)))))))
-
-;; TODO (?)
+(defmethod apply-form :order-by [[_ subclauses]]
+  (vec (for [{:keys [field direction]} subclauses]
+         `(order ~(if (= (:source field) :aggregation)
+                    ;; order by an aggregate Field
+                    (let [{:keys [aggregation-type]} (:aggregation (:query qp/*query*))]
+                      `(raw ~(name aggregation-type)))
+
+                    ;; order by a normal Field
+                    (formatted field))
+                 ~(case direction
+                    :ascending  :ASC
+                    :descending :DESC)))))
+
+;; TODO - page can be preprocessed away -- converted to a :limit clause and an :offset clause
+;; implement this at some point.
 (defmethod apply-form :page [[_ {:keys [items page]}]]
   {:pre [(integer? items)
          (> items 0)

src/metabase/driver/query_processor/expand.clj

@@ -49,6 +49,7 @@
          parse-breakout
          parse-fields
          parse-filter
+         parse-order-by
          with-resolved-fields)
 
 ;; ## -------------------- Protocols --------------------
@@ -75,7 +76,8 @@
                                               :aggregation (parse-aggregation (:aggregation %))
                                               :breakout    (parse-breakout    (:breakout %))
                                               :fields      (parse-fields      (:fields %))
-                                              :filter      (parse-filter      (:filter %)))
+                                              :filter      (parse-filter      (:filter %))
+                                              :order-by    (parse-order-by    (:order_by %)))
                                        (m/filter-vals identity))))
 
 (defn expand
@@ -83,11 +85,6 @@
   [query-dict]
   (with-resolved-fields parse query-dict))
 
-(defn expand-filter
-  "Expand a `filter` clause."
-  [filter-clause]
-  (with-resolved-fields parse-filter filter-clause))
-
 
 ;; ## -------------------- Field + Value --------------------
 
@@ -280,3 +277,25 @@
                                      :subclauses    (mapv parse-filter-subclause subclauses)})
   subclause            (map->Filter {:compound-type :simple
                                      :subclauses    [(parse-filter-subclause subclause)]}))
+
+
+;; ## -------------------- Order-By --------------------
+
+(defrecord OrderByAggregateField [^Keyword source  ; e.g. :aggregation
+                                  ^Integer index]) ; e.g. 0
+
+(defrecord OrderBySubclause [^Field   field       ; or aggregate Field?
+                             ^Keyword direction]) ; either :ascending or :descending
+
+(defn- parse-order-by-direction [direction]
+  (case direction
+    "ascending"  :ascending
+    "descending" :descending))
+
+(defparser parse-order-by-subclause
+  [["aggregation" index] direction]      (->OrderBySubclause (->OrderByAggregateField :aggregation index)
+                                                             (parse-order-by-direction direction))
+  [(field-id :guard integer?) direction] (->OrderBySubclause (ph field-id)
+                                                             (parse-order-by-direction direction)))
+(defparser parse-order-by
+  subclauses (mapv parse-order-by-subclause subclauses))