test fixes

src/metabase/driver/query_processor.clj

@@ -69,7 +69,7 @@
     (qp (if (or (not (= ag-type :rows)) breakout fields) query
             (-> query
                 (assoc-in [:query :fields-is-implicit] true)
-                (assoc-in [:query :fields] (->> (sel :many :fields [Field :name :base_type :special_type :table_id], :table_id source-table-id, :active true,
+                (assoc-in [:query :fields] (->> (sel :many :fields [Field :name :base_type :special_type :table_id :id], :table_id source-table-id, :active true,
                                                      :preview_display true, :field_type [not= "sensitive"], :parent_id nil, (k/order :position :asc), (k/order :id :desc))
                                                 (map expand/rename-mb-field-keys)
                                                 (map expand/map->Field)
@@ -175,12 +175,13 @@
 (defn- limit
   "Add an implicit `limit` clause to queries with `rows` aggregations, and limit the maximum number of rows that can be returned in post-processing."
   [qp]
-  (fn [{{{ag-type :aggregation-type} :aggregation, limit :limit} :query, :as query}]
+  (fn [{{{ag-type :aggregation-type} :aggregation, :keys [limit page]} :query, :as query}]
     (let [query   (cond-> query
                     (and (not limit)
+                         (not page)
                          (= ag-type :rows)) (assoc-in [:query :limit] max-result-bare-rows))
           results (qp query)]
-      (update-in results [:rows] (partial take max-result-rows)))))
+      (update results :rows (partial take max-result-rows)))))
 
 
 (defn- pre-log-query [qp]

src/metabase/driver/query_processor/expand.clj

@@ -110,15 +110,17 @@
                 (not (every? nil? clause))))))
 
 (defn- parse [query-dict]
-  (update-in query-dict [:query] #(-<> (assoc %
-                                              :aggregation (parse-aggregation (:aggregation %))
-                                              :breakout    (parse-breakout    (:breakout %))
-                                              :fields      (parse-fields      (:fields %))
-                                              :filter      (parse-filter      (:filter %))
-                                              :order_by    (parse-order-by    (:order_by %)))
-                                       (set/rename-keys <> {:order_by     :order-by
-                                                            :source_table :source-table})
-                                       (m/filter-vals non-empty-clause? <>))))
+  ;; TODO - we should parse the Page clause so we can validate it
+  ;; And convert to a limit / offset clauses
+  (update query-dict :query #(-<> (assoc %
+                                         :aggregation (parse-aggregation (:aggregation %))
+                                         :breakout    (parse-breakout    (:breakout %))
+                                         :fields      (parse-fields      (:fields %))
+                                         :filter      (parse-filter      (:filter %))
+                                         :order_by    (parse-order-by    (:order_by %)))
+                                  (set/rename-keys <> {:order_by     :order-by
+                                                       :source_table :source-table})
+                                  (m/filter-vals non-empty-clause? <>))))
 
 (defn rename-mb-field-keys
   "Rename the keys in a Metabase `Field` to match the format of those in Query Expander `Fields`."
@@ -350,15 +352,15 @@
                         ^Field field])
 
 (defparser parse-aggregation
-  ["rows"]              (->Aggregation :rows nil)
-  ["count"]             (->Aggregation :count nil)
-  ["avg" field-id]      (->Aggregation :avg (ph field-id))
-  ["count" field-id]    (->Aggregation :count (ph field-id))
-  ["distinct" field-id] (->Aggregation :distinct (ph field-id))
-  ["stddev" field-id]   (do (assert-driver-supports :standard-deviation-aggregations)
-                            (->Aggregation :stddev (ph field-id)))
-  ["sum" field-id]      (->Aggregation :sum (ph field-id))
-  ["cum_sum" field-id]  (->Aggregation :cumulative-sum (ph field-id)))
+  ["rows"]                              (->Aggregation :rows nil)
+  ["count"]                             (->Aggregation :count nil)
+  ["avg" (field-id :guard Field?)]      (->Aggregation :avg (ph field-id))
+  ["count" (field-id :guard Field?)]    (->Aggregation :count (ph field-id))
+  ["distinct" (field-id :guard Field?)] (->Aggregation :distinct (ph field-id))
+  ["stddev" (field-id :guard Field?)]   (do (assert-driver-supports :standard-deviation-aggregations)
+                                            (->Aggregation :stddev (ph field-id)))
+  ["sum" (field-id :guard Field?)]      (->Aggregation :sum (ph field-id))
+  ["cum_sum" (field-id :guard Field?)]  (->Aggregation :cumulative-sum (ph field-id)))
 
 
 ;; ## -------------------- Breakout --------------------

test/metabase/api/session_test.clj

 (ns metabase.api.session-test
   "Tests for /api/session"
   (:require [expectations :refer :all]
-            [korma.core :refer :all]
             [metabase.db :refer :all]
             [metabase.http-client :refer :all]
             (metabase.models [session :refer [Session]]