Adopt middleware pattern a la Compojure for QP

src/metabase/driver.clj

@@ -132,20 +132,7 @@
 (defn process-query
   "Process a structured or native query, and return the result."
   [query]
-  {:pre [(map? query)]}
-  (try
-    (let [driver  (database-id->driver (:database query))]
-      (binding [qp/*query*            query
-                qp/*internal-context* (atom {})
-                qp/*driver*           driver]
-        (let [query   (qp/preprocess query)
-              results (binding [qp/*query* query]
-                        (i/process-query driver (dissoc-in query [:query :cum_sum])))] ; strip out things that individual impls don't need to know about / deal with
-          (qp/post-process driver query results))))
-    (catch Throwable e
-      (.printStackTrace e)
-      {:status :failed
-       :error  (.getMessage e)})))
+  (qp/process (database-id->driver (:database query)) query))
 
 
 ;; ## Query Execution Stuff

src/metabase/driver/generic_sql/query_processor.clj

@@ -31,31 +31,35 @@
                  (second (re-find (:uncastify-timestamp-regex qp/*driver*) column-name))
                  column-name))))
 
+(def ^:dynamic ^:private *query* nil)
+
 (defn process-structured
   "Convert QUERY into a korma `select` form, execute it, and annotate the results."
   [{{:keys [source-table]} :query, database :database, :as query}]
-  (try
-    ;; Process the expanded query and generate a korma form
-    (let [korma-form `(let [entity# (korma-entity ~database ~source-table)]
-                        (select entity# ~@(->> (map apply-form (:query query))
-                                               (filter identity)
-                                               (mapcat #(if (vector? %) % [%])))))]
-
-      ;; Log generated korma form
-      (when (config/config-bool :mb-db-logging)
-        (log-korma-form korma-form))
-
-      ;; Now eval the korma form. Then annotate the results
-      ;; TODO - why does this happen within the individual drivers still? Annotate should be moved out
-      (let [results (eval korma-form)]
-        (qp/annotate query results uncastify)))
-
-    (catch java.sql.SQLException e
-      (let [^String message (or (->> (.getMessage e)                            ; error message comes back like "Error message ... [status-code]" sometimes
-                                          (re-find  #"(?s)(^.*)\s+\[[\d-]+\]$") ; status code isn't useful and makes unit tests hard to write so strip it off
-                                          second)                               ; (?s) = Pattern.DOTALL - tell regex `.` to match newline characters as well
-                                (.getMessage e))]
-        (throw (Exception. message))))))
+  (binding [*query* query]
+    (try
+      ;; Process the expanded query and generate a korma form
+      (let [korma-form `(let [entity# (korma-entity ~database ~source-table)]
+                          (select entity# ~@(->> (map apply-form (:query query))
+                                                 (filter identity)
+                                                 (mapcat #(if (vector? %) % [%])))))]
+
+        ;; Log generated korma form
+        (when (config/config-bool :mb-db-logging)
+          (log-korma-form korma-form))
+
+        ;; Now eval the korma form. Then annotate the results
+        ;; TODO - why does this happen within the individual drivers still? Annotate should be moved out
+        (let [results (eval korma-form)]
+          {:results      results
+           :uncastify-fn uncastify}))
+
+      (catch java.sql.SQLException e
+        (let [^String message (or (->> (.getMessage e)                            ; error message comes back like "Error message ... [status-code]" sometimes
+                                       (re-find  #"(?s)(^.*)\s+\[[\d-]+\]$") ; status code isn't useful and makes unit tests hard to write so strip it off
+                                       second)                               ; (?s) = Pattern.DOTALL - tell regex `.` to match newline characters as well
+                                  (.getMessage e))]
+          (throw (Exception. message)))))))
 
 (defn process-and-run
   "Process and run a query and return results."
@@ -100,7 +104,7 @@
   ;; e.g. the ["aggregation" 0] fields we allow in order-by
   OrderByAggregateField
   (formatted [_]
-    (let [{:keys [aggregation-type]} (:aggregation (:query qp/*query*))] ; determine the name of the aggregation field
+    (let [{:keys [aggregation-type]} (:aggregation (:query *query*))] ; determine the name of the aggregation field
       `(raw ~(case aggregation-type
                :avg      "\"avg\""
                :count    "\"count\""
@@ -136,7 +140,7 @@
 
     ;; Add fields form only for fields that weren't specified in :fields clause -- we don't want to include it twice, or korma will barf
     (fields ~@(->> fields
-                   (filter (partial (complement contains?) (set (:fields (:query qp/*query*)))))
+                   (filter (partial (complement contains?) (set (:fields (:query *query*)))))
                    (map formatted)))])
 
 

src/metabase/driver/mongo/query_processor.clj

@@ -11,7 +11,7 @@
             [metabase.db :refer :all]
             [metabase.driver :as driver]
             (metabase.driver [interface :as i]
-                             [query-processor :as qp :refer [*query*]])
+                             [query-processor :as qp])
             [metabase.driver.mongo.util :refer [with-mongo-connection *mongo-connection* values->base-type]]
             [metabase.models.field :refer [Field]]
             [metabase.util :as u])
@@ -21,8 +21,6 @@
            (org.bson.types ObjectId)))
 
 (declare apply-clause
-         annotate-native-results
-         annotate-results
          eval-raw-command
          process-structured
          process-and-run-structured)
@@ -30,20 +28,23 @@
 
 ;; # DRIVER QP INTERFACE
 
+(def ^:dynamic ^:private *query* nil)
+
 (defn process-and-run
   "Process and run a MongoDB QUERY."
   [{query-type :type, database :database, :as query}]
-  (with-mongo-connection [_ database]
-    (case (keyword query-type)
-      :query (let [generated-query (process-structured (:query query))]
-               (when-not qp/*disable-qp-logging*
-                 (log/debug (color/magenta "\n\n******************** Generated Monger Query: ********************\n"
-                                           (u/pprint-to-str generated-query)
-                                           "\n*****************************************************************\n")))
-               (->> (eval generated-query)
-                    (annotate-results query)))
-      :native (->> (eval-raw-command (:query (:native query)))
-                   annotate-native-results))))
+  (binding [*query* query]
+    (with-mongo-connection [_ database]
+      (case (keyword query-type)
+        :query (let [generated-query (process-structured (:query query))]
+                 (when-not qp/*disable-qp-logging*
+                   (log/debug (color/magenta "\n\n******************** Generated Monger Query: ********************\n"
+                                             (u/pprint-to-str generated-query)
+                                             "\n*****************************************************************\n")))
+                 {:results (eval generated-query)})
+        :native (let [results (eval-raw-command (:query (:native query)))]
+                  {:results (if (sequential? results) results
+                                [results])})))))
 
 
 ;; # NATIVE QUERY PROCESSOR
@@ -62,15 +63,6 @@
       (let [{result "retval"} (PersistentArrayMap/create (.toMap result))]
         result)))
 
-(defn annotate-native-results
-  "Package up the results in the way the frontend expects."
-  [results]
-  (if-not (sequential? results) (annotate-native-results [results])
-          {:status    :completed
-           :row_count (count results)
-           :data      {:rows    results
-                       :columns (keys (first results))}}))
-
 
 ;; # STRUCTURED QUERY PROCESSOR
 
@@ -226,15 +218,6 @@
         (match-aggregation aggregation))))
 
 
-;; ## ANNOTATION
-
-;; TODO - This is similar to the implementation in generic-sql; can we combine them and move it into metabase.driver.query-processor?
-(defn annotate-results
-  "Add column information, `row_count`, etc. to the results of a Mongo QP query."
-  [query results]
-  (qp/annotate query results))
-
-
 ;; ## CLAUSE APPLICATION 2.0
 
 (def ^:private clauses

src/metabase/driver/query_processor/expand.clj

@@ -100,16 +100,21 @@
   "Bound to an atom containing a set when a parsing function is ran"
   nil)
 
+(defn rename-mb-field-keys
+  "Rename the keys in a Metabase `Field` to match the format of those in Query Expander `Fields`."
+  [field]
+  (set/rename-keys field {:id           :field-id
+                          :name         :field-name
+                          :special_type :special-type
+                          :base_type    :base-type
+                          :table_id     :table-id}))
+
 (defn- resolve-fields
   "Resolve the `Fields` in an EXPANDED-QUERY-DICT."
   [expanded-query-dict field-ids]
   (if-not (seq field-ids) expanded-query-dict ; No need to do a DB call or walk expanded-query-dict if we didn't see any Field IDs
           (let [fields (->> (sel :many :id->fields [field/Field :name :base_type :special_type :table_id] :id [in field-ids])
-                            (m/map-vals #(set/rename-keys % {:id           :field-id
-                                                             :name         :field-name
-                                                             :special_type :special-type
-                                                             :base_type    :base-type
-                                                             :table_id     :table-id})))]
+                            (m/map-vals rename-mb-field-keys))]
             ;; This is performed depth-first so we don't end up walking the newly-created Field/Value objects
             ;; they may have nil values; this was we don't have to write an implementation of resolve-field for nil
             (walk/postwalk #(resolve-field % fields) expanded-query-dict))))
@@ -238,9 +243,8 @@
 
 ;; ## -------------------- Breakout --------------------
 
-;; Breakout + Fields clauses are just regular vectors
-;; TODO - might need to change this if we want to add any special
-;; functionality
+;; Breakout + Fields clauses are just regular vectors of Fields
+
 (defparser parse-breakout
   field-ids (mapv ph field-ids))
 

test/metabase/driver/query_processor_test.clj

@@ -8,7 +8,8 @@
                              [table :refer [Table]])
             [metabase.test.data :refer :all]
             (metabase.test.data [dataset-definitions :as defs]
-                                [datasets :as datasets :refer [*dataset*]])))
+                                [datasets :as datasets :refer [*dataset*]])
+            [metabase.util :as u]))
 
 
 
@@ -538,7 +539,7 @@
 ;; Apply limit-max-result-rows to an infinite sequence and make sure it gets capped at `max-result-rows`
 (expect max-result-rows
   (count (->> {:rows (repeat [:ok])}
-              limit-max-result-rows
+              ((u/runtime-resolved-fn 'metabase.driver.query-processor 'limit-max-result-rows))
               :rows)))