Remove roundtrip Timestamp->string->JodaTime

src/metabase/db/metadata_queries.clj

@@ -78,7 +78,7 @@
 (defn field-values
   "Return all the values of FIELD for QUERY."
   [field query]
-  (->> (qp/process-query
+  (->> (qp/process-query-no-format-rows
          {:type     :query
           :database (db-id field)
           :query    (merge {:fields       [[:field-id (:id field)]]
@@ -91,7 +91,7 @@
 (defn query-values
   "Return all values for QUERY."
   [db-id query]
-  (-> (qp/process-query
+  (-> (qp/process-query-no-format-rows
         {:type     :query
          :database db-id
          :query    query})

src/metabase/feature_extraction/feature_extractors.clj

@@ -4,7 +4,6 @@
             [clj-time
              [coerce :as t.coerce]
              [core :as t]
-             [format :as t.format]
              [periodic :as t.periodic]]
             [kixi.stats
              [core :as stats]
@@ -367,8 +366,8 @@
    (redux/pre-step
     (redux/fuse {:linear-regression (stats/simple-linear-regression first second)
                  :series            conj})
-    (fn [[x y]]
-      [(-> x t.format/parse to-double) y]))
+    (fn [[^java.sql.Timestamp x y]]
+      [(.getTime x) y]))
    (merge-juxt
     (field-metadata-extractor field)
     (fn [{:keys [series linear-regression]}]
@@ -464,24 +463,37 @@
     (field-metadata-extractor field)
     histogram-extractor)))
 
-(defn- quarter
-  [dt]
-  (-> dt t/month (/ 3) Math/ceil long))
+(defprotocol Quarter
+  (quarter [dt] "Return which quarter (1-4) given date-like object falls into."))
+
+(extend-type java.sql.Timestamp
+  Quarter
+  (quarter [dt]
+    (-> dt .getMonth (* 0.33) Math/ceil long)))
+
+(extend-type org.joda.time.DateTime
+  Quarter
+  (quarter [dt]
+    (-> dt t/month (* 0.33) Math/ceil long)))
 
 (defmethod feature-extractor DateTime
   [_ field]
   (redux/post-complete
-   (redux/pre-step
-    (redux/fuse {:histogram         (redux/pre-step h/histogram t.coerce/to-long)
-                 :histogram-hour    (redux/pre-step h/histogram-categorical
-                                                    (stats/somef t/hour))
-                 :histogram-day     (redux/pre-step h/histogram-categorical
-                                                    (stats/somef t/day-of-week))
-                 :histogram-month   (redux/pre-step h/histogram-categorical
-                                                    (stats/somef t/month))
-                 :histogram-quarter (redux/pre-step h/histogram-categorical
-                                                    (stats/somef quarter))})
-    t.format/parse)
+   (redux/fuse {:histogram         (redux/pre-step
+                                    h/histogram
+                                    (memfn ^java.sql.Timestamp getTime))
+                :histogram-hour    (redux/pre-step
+                                    h/histogram-categorical
+                                    (memfn ^java.sql.Timestamp getHours))
+                :histogram-day     (redux/pre-step
+                                    h/histogram-categorical
+                                    (memfn ^java.sql.Timestamp getDay))
+                :histogram-month   (redux/pre-step
+                                    h/histogram-categorical
+                                    (memfn ^java.sql.Timestamp getMonth))
+                :histogram-quarter (redux/pre-step
+                                    h/histogram-categorical
+                                    quarter)})
    (merge-juxt
     histogram-extractor
     (field-metadata-extractor field)

src/metabase/query_processor.clj

@@ -67,8 +67,39 @@
 ;; PRE-PROCESSING fns are applied from bottom to top, and POST-PROCESSING from top to bottom;
 ;; the easiest way to wrap your head around this is picturing a the query as a ball being thrown in the air
 ;; (up through the preprocessing fns, back down through the post-processing ones)
-(defn- qp-pipeline
-  "Construct a new Query Processor pipeline with F as the final 'piviotal' function. e.g.:
+(def ^:private qp-pipeline
+  [dev/guard-multiple-calls
+   mbql-to-native/mbql->native                      ; ▲▲▲ NATIVE-ONLY POINT ▲▲▲ Query converted from MBQL to native here; all functions *above* will only see the native query
+   annotate-and-sort/annotate-and-sort
+   perms/check-query-permissions
+   log-query/log-expanded-query
+   dev/check-results-format
+   limit/limit
+   cumulative-ags/handle-cumulative-aggregations
+   format-rows/format-rows
+   binning/update-binning-strategy
+   results-metadata/record-and-return-metadata!
+   resolve/resolve-middleware
+   add-dim/add-remapping
+   implicit-clauses/add-implicit-clauses
+   source-table/resolve-source-table-middleware
+   expand/expand-middleware                         ; ▲▲▲ QUERY EXPANSION POINT  ▲▲▲ All functions *above* will see EXPANDED query during PRE-PROCESSING
+   row-count-and-status/add-row-count-and-status    ; ▼▼▼ RESULTS WRAPPING POINT ▼▼▼ All functions *below* will see results WRAPPED in `:data` during POST-PROCESSING
+   parameters/substitute-parameters
+   expand-macros/expand-macros
+   driver-specific/process-query-in-context         ; (drivers can inject custom middleware if they implement IDriver's `process-query-in-context`)
+   add-settings/add-settings
+   resolve-driver/resolve-driver                    ; ▲▲▲ DRIVER RESOLUTION POINT ▲▲▲ All functions *above* will have access to the driver during PRE- *and* POST-PROCESSING
+   fetch-source-query/fetch-source-query
+   log-query/log-initial-query
+   cache/maybe-return-cached-results
+   log-query/log-results-metadata
+   catch-exceptions/catch-exceptions])
+  ;; ▼▼▼ POST-PROCESSING ▼▼▼  happens from TOP-TO-BOTTOM, e.g. the results of `f` are (eventually) passed to `limit`
+;; ▲▲▲ PRE-PROCESSING ▲▲▲ happens from BOTTOM-TO-TOP, e.g. the results of `expand-macros` are (eventually) passed to `expand-resolve`
+
+(defn- apply-pipeline
+  "Construct a new Query Processor using `pipeline` with `f` as the final 'piviotal' function. e.g.:
 
      All PRE-PROCESSING (query) --> F --> All POST-PROCESSING (result)
 
@@ -76,45 +107,19 @@
 
      (post-process (f (pre-process query)))
 
-   Normally F is something that runs the query, like the `execute-query` function above, but this can be swapped out when we want to do things like
+   Normally `f` is something that runs the query, like the `execute-query` function above, but this can be swapped out when we want to do things like
    process a query without actually running it."
-  [f]
-  ;; ▼▼▼ POST-PROCESSING ▼▼▼  happens from TOP-TO-BOTTOM, e.g. the results of `f` are (eventually) passed to `limit`
-  (-> f
-      dev/guard-multiple-calls
-      mbql-to-native/mbql->native                      ; ▲▲▲ NATIVE-ONLY POINT ▲▲▲ Query converted from MBQL to native here; all functions *above* will only see the native query
-      annotate-and-sort/annotate-and-sort
-      perms/check-query-permissions
-      log-query/log-expanded-query
-      dev/check-results-format
-      limit/limit
-      cumulative-ags/handle-cumulative-aggregations
-      format-rows/format-rows
-      binning/update-binning-strategy
-      results-metadata/record-and-return-metadata!
-      resolve/resolve-middleware
-      add-dim/add-remapping
-      implicit-clauses/add-implicit-clauses
-      source-table/resolve-source-table-middleware
-      expand/expand-middleware                         ; ▲▲▲ QUERY EXPANSION POINT  ▲▲▲ All functions *above* will see EXPANDED query during PRE-PROCESSING
-      row-count-and-status/add-row-count-and-status    ; ▼▼▼ RESULTS WRAPPING POINT ▼▼▼ All functions *below* will see results WRAPPED in `:data` during POST-PROCESSING
-      parameters/substitute-parameters
-      expand-macros/expand-macros
-      driver-specific/process-query-in-context         ; (drivers can inject custom middleware if they implement IDriver's `process-query-in-context`)
-      add-settings/add-settings
-      resolve-driver/resolve-driver                    ; ▲▲▲ DRIVER RESOLUTION POINT ▲▲▲ All functions *above* will have access to the driver during PRE- *and* POST-PROCESSING
-      fetch-source-query/fetch-source-query
-      log-query/log-initial-query
-      cache/maybe-return-cached-results
-      log-query/log-results-metadata
-      catch-exceptions/catch-exceptions))
-;; ▲▲▲ PRE-PROCESSING ▲▲▲ happens from BOTTOM-TO-TOP, e.g. the results of `expand-macros` are (eventually) passed to `expand-resolve`
+  [pipeline f]
+  (reduce (fn [acc middleware]
+            (middleware acc))
+          f
+          pipeline))
 
 (defn query->native
   "Return the native form for QUERY (e.g. for a MBQL query on Postgres this would return a map containing the compiled SQL form)."
   {:style/indent 0}
   [query]
-  (let [results ((qp-pipeline identity) query)]
+  (let [results ((apply-pipeline qp-pipeline identity) query)]
     (or (get-in results [:data :native_form])
         (throw (ex-info "No native form returned."
                  results)))))
@@ -123,7 +128,14 @@
   "A pipeline of various QP functions (including middleware) that are used to process MB queries."
   {:style/indent 0}
   [query]
-  ((qp-pipeline execute-query) query))
+  ((apply-pipeline qp-pipeline execute-query) query))
+
+(defn process-query-no-format-rows
+  "Same as `process-query` except it skips `format-rows/format-rows`"
+  {:style/indent 0}
+  [query]
+  (let [pipeline (remove #{format-rows/format-rows} qp-pipeline)]
+    ((apply-pipeline pipeline execute-query) query)))
 
 (def ^{:arglists '([query])} expand
   "Expand a QUERY the same way it would normally be done as part of query processing.