Remove use of Q macro in metabase.driver.query-processor-test

src/metabase/driver/generic_sql/query_processor.clj

@@ -18,7 +18,7 @@
            (metabase.driver.query_processor.interface DateTimeField
                                                       DateTimeValue
                                                       Field
-                                                      OrderByAggregateField
+                                                      AgFieldRef
                                                       RelativeDateTimeValue
                                                       Value)))
 
@@ -52,7 +52,7 @@
            field))))
 
   ;; e.g. the ["aggregation" 0] fields we allow in order-by
-  OrderByAggregateField
+  AgFieldRef
   (formatted
     ([this]
      (formatted this false))

src/metabase/driver/h2.clj

@@ -148,7 +148,7 @@
     :day-of-week     (k/sqlfn :DAY_OF_WEEK expr)
     :day-of-month    (k/sqlfn :DAY_OF_MONTH expr)
     :day-of-year     (k/sqlfn :DAY_OF_YEAR expr)
-    :week            (trunc-with-format "yyyyww" expr) ; ww = week of year
+    :week            (trunc-with-format "YYYYww" expr) ; Y = week year; w = week in year
     :week-of-year    (kx/week expr)
     :month           (trunc-with-format "yyyyMM" expr)
     :month-of-year   (kx/month expr)

src/metabase/driver/mongo/query_processor.clj

@@ -28,7 +28,7 @@
            (metabase.driver.query_processor.interface DateTimeField
                                                       DateTimeValue
                                                       Field
-                                                      OrderByAggregateField
+                                                      AgFieldRef
                                                       RelativeDateTimeValue
                                                       Value)))
 
@@ -111,7 +111,7 @@
   (->initial-rvalue [this]
     (str \$ (field->name this ".")))
 
-  OrderByAggregateField
+  AgFieldRef
   (->lvalue [_]
     (let [{:keys [aggregation-type]} (:aggregation (:query *query*))]
       (case aggregation-type

src/metabase/driver/query_processor/expand.clj

@@ -10,12 +10,12 @@
             [metabase.models.table :refer [Table]]
             [metabase.util :as u]
             [schema.core :as s])
-  (:import (metabase.driver.query_processor.interface BetweenFilter
+  (:import (metabase.driver.query_processor.interface AgFieldRef
+                                                      BetweenFilter
                                                       ComparisonFilter
                                                       CompoundFilter
                                                       EqualityFilter
                                                       FieldPlaceholder
-                                                      OrderByAggregateField
                                                       RelativeDatetime
                                                       StringFilter
                                                       ValuePlaceholder)))
@@ -71,25 +71,26 @@
 ;;; # ------------------------------------------------------------ Clause Handlers ------------------------------------------------------------
 
 ;; TODO - check that there's a matching :aggregation clause in the query ?
-(s/defn ^:always-validate aggregate-field :- OrderByAggregateField
+(s/defn ^:always-validate aggregate-field :- AgFieldRef
   "Aggregate field referece, e.g. for use in an `order-by` clause.
 
      (query (aggregate (count))
             (order-by [(aggregate-field 0) :ascending])) ; order by :count"
   [index :- s/Int]
-  (i/map->OrderByAggregateField {:index index}))
+  (i/map->AgFieldRef {:index index}))
 
 (s/defn ^:always-validate field :- i/FieldPlaceholderOrAgRef
   "Generic reference to a `Field`. F can be an integer Field ID, or various other forms like `fk->` or `aggregation`."
   [f]
   (cond
-    (map?     f) (i/map->FieldPlaceholder f)
-    (integer? f) (i/map->FieldPlaceholder {:field-id f})
-    (vector?  f) (let [[token & args] f]
-                   (if (core/= (normalize-token token) :aggregation)
-                     (apply aggregate-field args)
-                     (apply-fn-for-token token args)))
-    :else        (throw (Exception. (str "Invalid field: " f)))))
+    (instance? AgFieldRef f) f
+    (map?     f)             (i/map->FieldPlaceholder f)
+    (integer? f)             (i/map->FieldPlaceholder {:field-id f})
+    (vector?  f)             (let [[token & args] f]
+                               (if (core/= (normalize-token token) :aggregation)
+                                 (apply aggregate-field args)
+                                 (apply-fn-for-token token args)))
+    :else                    (throw (Exception. (str "Invalid field: " f)))))
 
 (s/defn ^:always-validate datetime-field :- FieldPlaceholder
   "Reference to a `DateTimeField`. This is just a `Field` reference with an associated datetime UNIT."
@@ -341,15 +342,21 @@
   [outer-query]
   (update outer-query :query expand-inner))
 
+(defn validate-query
+  "Check that a given query is valid, returning it as-is if so."
+  [query]
+  (s/validate i/Query query))
+
 (defmacro query
   "Build a query by threading an (initially empty) map through each form in BODY with `->`.
    The final result is validated against the `Query` schema."
   {:style/indent 0}
   [& body]
-  `(s/validate i/Query (-> {} ~@body)))
+  `(-> {}
+       ~@body
+       validate-query))
 
 (s/defn ^:always-validate run-query* [query :- i/Query]
-  #_(println "query:\n" (u/pprint-to-str 'cyan query))
   (let [db-id (db/sel :one :field [Table :db_id], :id (:source-table query))]
     (driver/process-query {:database db-id
                            :type     :query

src/metabase/driver/query_processor/interface.clj

@@ -105,9 +105,9 @@
                                fk-field-id   :- (s/maybe s/Int)
                                datetime-unit :- (s/maybe (apply s/enum datetime-field-units))])
 
-(s/defrecord OrderByAggregateField [index :- s/Int]) ; e.g. 0
+(s/defrecord AgFieldRef [index :- s/Int]) ; e.g. 0
 
-(def FieldPlaceholderOrAgRef (s/named (s/cond-pre FieldPlaceholder OrderByAggregateField) "Valid field (field ID or aggregate field reference)"))
+(def FieldPlaceholderOrAgRef (s/named (s/cond-pre FieldPlaceholder AgFieldRef) "Valid field (field ID or aggregate field reference)"))
 
 
 (s/defrecord RelativeDatetime [amount :- s/Int

test/metabase/test/data.clj

@@ -26,7 +26,7 @@
   ([]            (get-or-create-test-data-db! *data-loader*))
   ([data-loader] (get-or-create-database! data-loader defs/test-data)))
 
-(def ^:dynamic *get-db* get-or-create-test-data-db!)
+(def ^:dynamic ^:private *get-db* get-or-create-test-data-db!)
 
 (defn db
   "Return the current database.
@@ -34,13 +34,15 @@
   []
   (*get-db*))
 
+(defn do-with-db [db f]
+  (binding [*get-db* (constantly db)]
+    (f)))
+
 (defmacro with-db
   "Run body with DB as the current database.
    Calls to `db` and `id` use this value."
   [db & body]
-  `(let [db# ~db]
-     (binding [*get-db* (constantly db#)]
-       ~@body)))
+  `(do-with-db ~db (fn [] ~@body)))
 
 (defn format-name [nm]
   (i/format-name *data-loader* (name nm)))

test/metabase/test/data/datasets.clj

@@ -60,15 +60,15 @@
    that is used to store the `test-data` dataset when you call `load-data!`."
   (driver/engine->driver default-engine))
 
-
+(defn do-with-engine [engine f]
+  (binding [*engine*      engine
+            *data-loader* (driver/engine->driver engine)]
+    (f)))
 
 (defmacro with-engine
   "Bind `*data-loader*` to the dataset with ENGINE and execute BODY."
   [engine & body]
-  `(let [engine# ~engine]
-     (binding [*engine*      engine#
-               *data-loader* (driver/engine->driver engine#)]
-       ~@body)))
+  `(do-with-engine ~engine (fn [] ~@body)))
 
 (defmacro when-testing-engine
   "Execute BODY only if we're currently testing against ENGINE."
@@ -94,17 +94,23 @@
   "Generate a unit test that only runs if we're currently testing against ENGINE, and that binds `*data-loader*` to the current dataset."
   [engine expected actual]
   `(expect-when-testing-engine ~engine
-     (with-engine ~engine
-       ~expected)
-     (with-engine ~engine
-       ~actual)))
+     (with-engine ~engine ~expected)
+     (with-engine ~engine ~actual)))
 
 (defmacro expect-with-engines
   "Generate unit tests for all datasets in ENGINES; each test will only run if we're currently testing the corresponding dataset.
    `*data-loader*` is bound to the current dataset inside each test."
   [engines expected actual]
-  `(do ~@(for [engine (eval engines)]
-           `(expect-with-engine ~engine ~expected ~actual))))
+  ;; Make functions to get expected/actual so the code is only compiled one time instead of for every single driver
+  ;; speeds up loading of metabase.driver.query-processor-test significantly
+  (let [e (gensym "expected-")
+        a (gensym "actual-")]
+    `(let [~e (fn [] ~expected)
+           ~a (fn [] ~actual)]
+       ~@(for [engine (eval engines)]
+           `(expect-when-testing-engine ~engine
+              (do-with-engine ~engine ~e)
+              (do-with-engine ~engine ~a))))))
 
 (defmacro expect-with-all-engines
   "Generate unit tests for all valid datasets; each test will only run if we're currently testing the corresponding dataset.