Add a new report-timezone dynamic var, use it as a default [ci drivers]

This commit adds a new `*report-timezone*` dynamic var that should get
bound before dates get parsed (i.e. in a query pipeline
middleware). This allows an easier API `(->Timestamp my-date-string)`
while still ensuring the dates are interpretted correctly.

This is a step toward centralization of our timezone and date
conversion logic. As of this commit there are still separate/similar
logic for handling reporting timezone etc in Pulses, drivers etc.

Fixes #7584, fixes #6402

src/metabase/driver/bigquery.clj

@@ -149,21 +149,13 @@
                    (.setQuery query-string))]
      (google/execute (.query (.jobs client) project-id request)))))
 
-(def ^:private ^java.util.TimeZone default-timezone
-  (java.util.TimeZone/getDefault))
-
 (defn- parse-timestamp-str [s]
   ;; Timestamp strings either come back as ISO-8601 strings or Unix timestamps in µs, e.g. "1.3963104E9"
   (or
-   (du/->Timestamp s)
-   ;; If parsing as ISO-8601 fails parse as a double then convert to ms. Add the appropriate number of milliseconds to
-   ;; the number to convert it to the local timezone. We do this because the dates come back in UTC but we want the
-   ;; grouping to match the local time (HUH?) This gives us the same results as the other
-   ;; `has-questionable-timezone-support?` drivers. Not sure if this is actually desirable, but if it's not, it
-   ;; probably means all of those other drivers are doing it wrong
-   (du/->Timestamp (- (* (Double/parseDouble s) 1000)
-                      (.getDSTSavings default-timezone)
-                      (.getRawOffset  default-timezone)))))
+   (du/->Timestamp s time/utc)
+   ;; If parsing as ISO-8601 fails parse as a double then convert to ms. This is ms since epoch in UTC. By using
+   ;; `->Timestamp`, it will convert from ms in UTC to a timestamp object in the JVM timezone
+   (du/->Timestamp (* (Double/parseDouble s) 1000))))
 
 (def ^:private bigquery-time-format (tformat/formatter "HH:mm:SS" time/utc))
 

src/metabase/driver/mongo/query_processor.clj

@@ -18,7 +18,7 @@
              [collection :as mc]
              [operators :refer :all]])
   (:import java.sql.Timestamp
-           java.util.Date
+           [java.util Date TimeZone]
            [metabase.query_processor.interface AgFieldRef DateTimeField DateTimeValue Field FieldLiteral
             RelativeDateTimeValue Value]
            org.bson.types.ObjectId
@@ -425,7 +425,7 @@
     (into {} (for [[k v] row]
                {k (if (and (map? v)
                            (:___date v))
-                    (du/->Timestamp (:___date v))
+                    (du/->Timestamp (:___date v) (TimeZone/getDefault))
                     v)}))))
 
 

src/metabase/pulse/render.clj

@@ -646,7 +646,7 @@
   [render-type timezone card {:keys [rows cols] :as data}]
   (let [[x-axis-rowfn y-axis-rowfn] (graphing-columns card data)
         ft-row (if (datetime-field? (x-axis-rowfn cols))
-                 #(.getTime ^Date (du/->Timestamp %))
+                 #(.getTime ^Date (du/->Timestamp % timezone))
                  identity)
         rows   (if (> (ft-row (x-axis-rowfn (first rows)))
                       (ft-row (x-axis-rowfn (last rows))))

src/metabase/query_processor.clj

@@ -15,6 +15,7 @@
              [add-settings :as add-settings]
              [annotate-and-sort :as annotate-and-sort]
              [binning :as binning]
+             [bind-effective-timezone :as bind-timezone]
              [cache :as cache]
              [catch-exceptions :as catch-exceptions]
              [cumulative-aggregations :as cumulative-ags]
@@ -108,6 +109,7 @@
       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
+      bind-timezone/bind-effective-timezone
       fetch-source-query/fetch-source-query
       log-query/log-initial-query
       cache/maybe-return-cached-results
@@ -142,7 +144,8 @@
        expand-macros/expand-macros
        driver-specific/process-query-in-context
        resolve-driver/resolve-driver
-       fetch-source-query/fetch-source-query))
+       fetch-source-query/fetch-source-query
+       bind-timezone/bind-effective-timezone))
 ;; ▲▲▲ This only does PRE-PROCESSING, so it happens from bottom to top, eventually returning the preprocessed query
 ;; instead of running it
 

src/metabase/query_processor/middleware/bind_effective_timezone.clj

+(ns metabase.query-processor.middleware.bind-effective-timezone
+  (:require [metabase.util.date :as ud]))
+
+(defn bind-effective-timezone
+  "Middlware that ensures the report-timezone and data-timezone are bound based on the database being queried against"
+  [qp]
+  (fn [query]
+    (ud/with-effective-timezone (:database query)
+      (qp query))))

src/metabase/query_processor/middleware/parameters/sql.clj

@@ -20,6 +20,7 @@
            honeysql.types.SqlCall
            java.text.NumberFormat
            java.util.regex.Pattern
+           java.util.TimeZone
            metabase.models.field.FieldInstance))
 
 ;; The Basics:
@@ -45,9 +46,6 @@
 ;; can share one used somewhere else instead
 (def ^:private ^:dynamic *driver* nil)
 
-(def ^:private ^:dynamic *timezone* nil)
-
-
 ;; various record types below are used as a convenience for differentiating the different param types.
 
 ;; "Dimension" here means a "FIELD FILTER", e.g. something that expands to a clause like "some_field BETWEEN 1 AND 10"
@@ -299,7 +297,7 @@
 (s/defn ^:private relative-date-dimension-value->replacement-snippet-info :- ParamSnippetInfo
   [value]
   ;; TODO - get timezone from query dict
-  (-> (date-params/date-string->range value *timezone*)
+  (-> (date-params/date-string->range value (.getID du/*report-timezone*))
       map->DateRange
       ->replacement-snippet-info))
 
@@ -528,8 +526,7 @@
 (defn expand
   "Expand parameters inside a *SQL* QUERY."
   [query]
-  (binding [*driver*   (ensure-driver query)
-            *timezone* (get-in query [:settings :report-timezone])]
+  (binding [*driver*   (ensure-driver query)]
     (if (driver/driver-supports? *driver* :native-query-params)
       (update query :native expand-query-params (query->params-map query))
       query)))

src/metabase/query_processor/middleware/resolve.clj

@@ -10,6 +10,7 @@
             [metabase
              [db :as mdb]
              [util :as u]]
+            [metabase.util.date :as du]
             [metabase.models
              [database :refer [Database]]
              [field :as field]
@@ -229,11 +230,7 @@
 
   DateTimeField
   (parse-value [this value]
-    (let [tz                 (when-let [tz-id ^String (setting/get :report-timezone)]
-                               (TimeZone/getTimeZone tz-id))
-          parsed-string-date (some-> value
-                                     (du/str->date-time tz)
-                                     du/->Timestamp)]
+    (let [parsed-string-date (some-> value du/->Timestamp)]
       (cond
         parsed-string-date
         (s/validate DateTimeValue (i/map->DateTimeValue {:field this, :value parsed-string-date}))

src/metabase/sync/analyze/fingerprint.clj

@@ -15,7 +15,9 @@
              [sample :as sample]
              [text :as text]]
             [metabase.util :as u]
-            [metabase.util.schema :as su]
+            [metabase.util
+             [date :as du]
+             [schema :as su]]
             [schema.core :as s]
             [toucan.db :as db]))
 
@@ -176,8 +178,9 @@
   [database :- i/DatabaseInstance
    tables :- [i/TableInstance]
    log-progress-fn]
-  (apply merge-with + (for [table tables
-                            :let [result (fingerprint-fields! table)]]
-                        (do
-                          (log-progress-fn "fingerprint-fields" table)
-                          result))))
+  (du/with-effective-timezone database
+    (apply merge-with + (for [table tables
+                              :let [result (fingerprint-fields! table)]]
+                          (do
+                            (log-progress-fn "fingerprint-fields" table)
+                            result)))))

src/metabase/util/date.clj

@@ -4,38 +4,122 @@
              [core :as t]
              [format :as time]]
             [clojure.math.numeric-tower :as math]
-            [metabase.util :as u])
+            [clojure.tools.logging :as log]
+            [metabase.util :as u]
+            [puppetlabs.i18n.core :refer [trs]])
   (:import clojure.lang.Keyword
            [java.sql Time Timestamp]
            [java.util Calendar Date TimeZone]
-           org.joda.time.DateTime
+           [org.joda.time DateTime DateTimeZone]
            org.joda.time.format.DateTimeFormatter))
 
+(def ^{:tag     TimeZone
+       :dynamic true
+       :doc     "Timezone to be used when formatting timestamps for display or for the data (pre aggregation)"}
+  *report-timezone*)
+
+(def ^{:dynamic true
+       :doc     "The timezone of the data being queried. Today this is the same as the database timezone."
+       :tag     TimeZone}
+  *data-timezone*)
+
+(defprotocol ITimeZoneCoercible
+  "Coerce object to `java.util.TimeZone`"
+  (coerce-to-timezone ^TimeZone [this]
+    "Coerce `this` to `java.util.TimeZone`"))
+
+(extend-protocol ^:private ITimeZoneCoercible
+  String       (coerce-to-timezone [this]
+                 (TimeZone/getTimeZone this))
+  TimeZone     (coerce-to-timezone [this]
+                 this)
+  DateTimeZone (coerce-to-timezone [this]
+                 (.toTimeZone this)))
+
+(def ^TimeZone utc
+  "UTC TimeZone"
+  (coerce-to-timezone "UTC"))
+
+(def ^:private jvm-timezone
+  (delay (coerce-to-timezone (System/getProperty "user.timezone"))))
+
+(defn- warn-on-timezone-conflict
+  "Attempts to check the combination of report-timezone, jvm-timezone and data-timezone to determine of we have a
+  possible conflict. If one is found, warn the user."
+  [driver db ^TimeZone report-timezone ^TimeZone jvm-timezone ^TimeZone data-timezone]
+  ;; No need to check this if we don't have a data-timezone
+  (when (and data-timezone driver)
+    (let [jvm-data-tz-conflict? (not (.hasSameRules jvm-timezone data-timezone))]
+      (if ((resolve 'metabase.driver/driver-supports?) driver :set-timezone)
+        ;; This database could have a report-timezone configured, if it doesn't and the JVM and data timezones don't
+        ;; match, we should suggest that the user configure a report timezone
+        (when (and (not report-timezone)
+                   jvm-data-tz-conflict?)
+          (log/warn (str (trs "Possible imezone conflict found on database {0}." (:name db))
+                         (trs "JVM timezone is {0} and detected database timezone is {1}."
+                              (.getID jvm-timezone) (.getID data-timezone))
+                         (trs "Configure a report timezone to ensure proper date and time conversions."))))
+        ;; This database doesn't support a report timezone, check the JVM and data timezones, if they don't match,
+        ;; warn the user
+        (when jvm-data-tz-conflict?
+          (log/warn (str (trs "Possible timezone conflict found on database {0}." (:name db))
+                         (trs "JVM timezone is {0} and detected database timezone is {1}."
+                              (.getID jvm-timezone) (.getID data-timezone)))))))))
+
+(defn call-with-effective-timezone
+  "Invokes `f` with `*report-timezone*` and `*data-timezone*` bound for the given `db`"
+  [db f]
+  (let [driver    ((resolve 'metabase.driver/->driver) db)
+        report-tz (when-let [report-tz-id (and driver ((resolve 'metabase.driver/report-timezone-if-supported) driver))]
+                    (coerce-to-timezone report-tz-id))
+        data-tz   (some-> db :timezone coerce-to-timezone)
+        jvm-tz    @jvm-timezone]
+    (warn-on-timezone-conflict driver db report-tz jvm-tz data-tz)
+    (binding [*report-timezone* (or report-tz jvm-tz)
+              *data-timezone*   data-tz]
+      (f))))
+
+(defmacro with-effective-timezone
+  "Runs `body` with `*report-timezone*` and `*data-timezone*` configured using the given `db`"
+  [db & body]
+  `(call-with-effective-timezone ~db (fn [] ~@body)))
+
 (defprotocol ITimestampCoercible
   "Coerce object to a `java.sql.Timestamp`."
-  (->Timestamp ^java.sql.Timestamp [this]
+  (coerce-to-timestamp ^java.sql.Timestamp [this] [this timezone-coercible]
     "Coerce this object to a `java.sql.Timestamp`. Strings are parsed as ISO-8601."))
 
-(declare str->date-time)
-
-(extend-protocol ITimestampCoercible
-  nil       (->Timestamp [_]
+(extend-protocol ^:private ITimestampCoercible
+  nil       (coerce-to-timestamp [_]
               nil)
-  Timestamp (->Timestamp [this]
+  Timestamp (coerce-to-timestamp [this]
               this)
-  Date       (->Timestamp [this]
-               (Timestamp. (.getTime this)))
+  Date      (coerce-to-timestamp
+              [this]
+              (coerce/to-timestamp (coerce/from-date this)))
   ;; Number is assumed to be a UNIX timezone in milliseconds (UTC)
-  Number    (->Timestamp [this]
-              (Timestamp. this))
-  Calendar  (->Timestamp [this]
-              (->Timestamp (.getTime this)))
-  ;; Strings are expected to be in ISO-8601 format. `YYYY-MM-DD` strings *are* valid ISO-8601 dates.
-  String    (->Timestamp [this]
-              (->Timestamp (str->date-time this)))
-  DateTime  (->Timestamp [this]
-              (->Timestamp (.getMillis this))))
+  Number    (coerce-to-timestamp [this]
+              (coerce/to-timestamp (coerce/from-long (long this))))
+  Calendar  (coerce-to-timestamp [this]
+              (coerce-to-timestamp (.getTime this)))
+  DateTime  (coerce-to-timestamp [this]
+              (coerce/to-timestamp this)))
 
+(declare str->date-time)
+
+(defn ^Timestamp ->Timestamp
+  "Converts `coercible-to-ts` to a `java.util.Timestamp`. Requires a `coercible-to-tz` if converting a string. Leans
+  on clj-time to ensure correct conversions between the various types"
+  ([coercible-to-ts]
+   {:pre [(or (not (string? coercible-to-ts))
+              (and (string? coercible-to-ts) (bound? #'*report-timezone*)))]}
+   (->Timestamp coercible-to-ts *report-timezone*))
+  ([coercible-to-ts timezone]
+   {:pre [(or (not (string? coercible-to-ts))
+              (and (string? coercible-to-ts) timezone))]}
+   (if (string? coercible-to-ts)
+     (coerce-to-timestamp (str->date-time coercible-to-ts (coerce-to-timezone timezone)))
+     (coerce-to-timestamp coercible-to-ts))))
 
 (defprotocol IDateTimeFormatterCoercible
   "Protocol for converting objects to `DateTimeFormatters`."
@@ -62,7 +146,6 @@
   ^java.sql.Timestamp [date-format, ^String s]
   (->Timestamp (time/parse (->DateTimeFormatter date-format) s)))
 
-
 (defprotocol ISO8601
   "Protocol for converting objects to ISO8601 formatted strings."
   (->iso-8601-datetime ^String [this timezone-id]
@@ -141,7 +224,9 @@
   [^String s]
   (boolean (when (string? s)
              (u/ignore-exceptions
-               (->Timestamp s)))))
+               ;; Using UTC as the timezone here as it's `def`'d and the result of the parse is discarded, any
+               ;; timezone is fine here
+               (->Timestamp s utc)))))
 
 (defn ->Date
   "Coerece DATE to a `java.util.Date`."
@@ -150,7 +235,6 @@
   (^java.util.Date [date]
    (java.util.Date. (.getTime (->Timestamp date)))))
 
-
 (defn ->Calendar
   "Coerce DATE to a `java.util.Calendar`."
   (^java.util.Calendar []
@@ -163,7 +247,6 @@
    (doto (->Calendar date)
      (.setTimeZone (TimeZone/getTimeZone timezone-id)))))
 
-
 (defn relative-date
   "Return a new `Timestamp` relative to the current time using a relative date UNIT.
 
@@ -185,7 +268,6 @@
                        (* amount multiplier)))
      (->Timestamp cal))))
 
-
 (def ^:private ^:const date-extract-units
   #{:minute-of-hour :hour-of-day :day-of-week :day-of-month :day-of-year :week-of-year :month-of-year :quarter-of-year
     :year})
@@ -215,14 +297,14 @@
                                   3)))
        :year            (.get cal Calendar/YEAR)))))
 
-
 (def ^:private ^:const date-trunc-units
   #{:minute :hour :day :week :month :quarter :year})
 
 (defn- trunc-with-format [format-string date timezone-id]
   (->Timestamp (format-date (time/with-zone (time/formatter format-string)
                               (t/time-zone-for-id timezone-id))
-                            date)))
+                            date)
+               timezone-id))
 
 (defn- trunc-with-floor [date amount-ms]
   (->Timestamp (* (math/floor (/ (.getTime (->Timestamp date))
@@ -259,7 +341,6 @@
      :quarter (trunc-with-format (format-string-for-quarter date timezone-id) date timezone-id)
      :year    (trunc-with-format "yyyy-01-01'T'ZZ" date timezone-id))))
 
-
 (defn date-trunc-or-extract
   "Apply date bucketing with UNIT to DATE. DATE defaults to now."
   ([unit]

test/metabase/api/activity_test.clj

@@ -23,19 +23,19 @@
 ;; NOTE: timestamp matching was being a real PITA so I cheated a bit.  ideally we'd fix that
 (tt/expect-with-temp [Activity [activity1 {:topic     "install"
                                            :details   {}
-                                           :timestamp (du/->Timestamp "2015-09-09T12:13:14.888Z")}]
+                                           :timestamp (du/->Timestamp #inst "2015-09-09T12:13:14.888Z")}]
                       Activity [activity2 {:topic     "dashboard-create"
                                            :user_id   (user->id :crowberto)
                                            :model     "dashboard"
                                            :model_id  1234
                                            :details   {:description  "Because I can!"
                                                        :name         "Bwahahaha"}
-                                           :timestamp (du/->Timestamp "2015-09-10T18:53:01.632Z")}]
+                                           :timestamp (du/->Timestamp #inst "2015-09-10T18:53:01.632Z")}]
                       Activity [activity3 {:topic     "user-joined"
                                            :user_id   (user->id :rasta)
                                            :model     "user"
                                            :details   {}
-                                           :timestamp (du/->Timestamp "2015-09-10T05:33:43.641Z")}]]
+                                           :timestamp (du/->Timestamp #inst "2015-09-10T05:33:43.641Z")}]]
   [(match-$ (Activity (:id activity2))
      {:id           $
       :topic        "dashboard-create"

test/metabase/api/card_test.clj

@@ -141,15 +141,15 @@
                       ;; 3 was viewed most recently, followed by 4, then 1. Card 2 was viewed by a different user so
                       ;; shouldn't be returned
                       ViewLog  [_ {:model "card", :model_id card-1-id, :user_id (user->id :rasta)
-                                   :timestamp (du/->Timestamp "2015-12-01")}]
+                                   :timestamp (du/->Timestamp #inst "2015-12-01")}]
                       ViewLog  [_ {:model "card", :model_id card-2-id, :user_id (user->id :trashbird)
-                                   :timestamp (du/->Timestamp "2016-01-01")}]
+                                   :timestamp (du/->Timestamp #inst "2016-01-01")}]
                       ViewLog  [_ {:model "card", :model_id card-3-id, :user_id (user->id :rasta)
-                                   :timestamp (du/->Timestamp "2016-02-01")}]
+                                   :timestamp (du/->Timestamp #inst "2016-02-01")}]
                       ViewLog  [_ {:model "card", :model_id card-4-id, :user_id (user->id :rasta)
-                                   :timestamp (du/->Timestamp "2016-03-01")}]
+                                   :timestamp (du/->Timestamp #inst "2016-03-01")}]
                       ViewLog  [_ {:model "card", :model_id card-3-id, :user_id (user->id :rasta)
-                                   :timestamp (du/->Timestamp "2016-04-01")}]]
+                                   :timestamp (du/->Timestamp #inst "2016-04-01")}]]
   [card-3-id card-4-id card-1-id]
   (mapv :id ((user->client :rasta) :get 200 "card", :f :recent)))
 

test/metabase/driver/mysql_test.clj

@@ -99,8 +99,8 @@
     (tu/db-timezone-id)))
 
 
-(def before-daylight-savings (du/str->date-time "2018-03-10 10:00:00"))
-(def after-daylight-savings (du/str->date-time "2018-03-12 10:00:00"))
+(def before-daylight-savings (du/str->date-time "2018-03-10 10:00:00" du/utc))
+(def after-daylight-savings (du/str->date-time "2018-03-12 10:00:00" du/utc))
 
 (expect (#'mysql/timezone-id->offset-str "US/Pacific" before-daylight-savings) "-08:00")
 (expect (#'mysql/timezone-id->offset-str "US/Pacific" after-daylight-savings)  "-07:00")
@@ -114,18 +114,18 @@
 ;; make sure DateTime types generate appropriate SQL...
 ;; ...with no report-timezone set
 (expect
-  ["?" (du/->Timestamp "2018-01-03")]
+  ["?" (du/->Timestamp #inst "2018-01-03")]
   (tu/with-temporary-setting-values [report-timezone nil]
-    (hsql/format (sqlqp/->honeysql (MySQLDriver.) (du/->Timestamp "2018-01-03")))))
+    (hsql/format (sqlqp/->honeysql (MySQLDriver.) (du/->Timestamp #inst "2018-01-03")))))
 
 ;; ...with a report-timezone set
 (expect
   ["convert_tz('2018-01-03T00:00:00.000', '+00:00', '-08:00')"]
   (tu/with-temporary-setting-values [report-timezone "US/Pacific"]
-    (hsql/format (sqlqp/->honeysql (MySQLDriver.) (du/->Timestamp "2018-01-03")))))
+    (hsql/format (sqlqp/->honeysql (MySQLDriver.) (du/->Timestamp #inst "2018-01-03")))))
 
 ;; ...with a report-timezone set to the same as the system timezone (shouldn't need to do TZ conversion)
 (expect
   ["?" (du/->Timestamp #inst "2018-01-03")]
   (tu/with-temporary-setting-values [report-timezone "UTC"]
-    (hsql/format (sqlqp/->honeysql (MySQLDriver.) (du/->Timestamp "2018-01-03")))))
+    (hsql/format (sqlqp/->honeysql (MySQLDriver.) (du/->Timestamp #inst "2018-01-03")))))

test/metabase/http_client.clj

@@ -41,7 +41,8 @@
         (map? response) (->> response
                              (map (fn [[k v]]
                                     {k (cond
-                                         (contains? auto-deserialize-dates-keys k) (du/->Timestamp v)
+                                         ;; Our tests only run in UTC, parsing timestamp strings as UTC
+                                         (contains? auto-deserialize-dates-keys k) (du/->Timestamp v du/utc)
                                          (coll? v) (auto-deserialize-dates v)
                                          :else v)}))
                              (into {}))

test/metabase/models/session_test.clj

@@ -19,16 +19,16 @@
     (db/simple-insert-many! Session
       [{:id         "the-greatest-day-ever"
         :user_id    user-id
-        :created_at (du/->Timestamp "1980-10-19T05:05:05.000Z")}
+        :created_at (du/->Timestamp #inst "1980-10-19T05:05:05.000Z")}
        {:id         "even-more-greatness"
         :user_id    user-id
-        :created_at (du/->Timestamp "1980-10-19T05:08:05.000Z")}
+        :created_at (du/->Timestamp #inst "1980-10-19T05:08:05.000Z")}
        {:id         "the-world-of-bi-changes-forever"
         :user_id    user-id
-        :created_at (du/->Timestamp "2015-10-21")}
+        :created_at (du/->Timestamp #inst "2015-10-21")}
        {:id         "something-could-have-happened"
         :user_id    user-id
-        :created_at (du/->Timestamp "1999-12-31")}
+        :created_at (du/->Timestamp #inst "1999-12-31")}
        {:id         "now"
         :user_id    user-id
         :created_at (du/new-sql-timestamp)}])

test/metabase/query_processor/expand_resolve_test.clj

@@ -9,6 +9,7 @@
              [expand :as ql]
              [resolve :as resolve]
              [source-table :as source-table]]
+            [metabase.query-processor-test :as qpt]
             [metabase.test
              [data :as data :refer :all]
              [util :as tu]]
@@ -240,7 +241,7 @@
                                                                                     :type   {:type/DateTime {:earliest "2014-01-01T00:00:00.000Z"
                                                                                                              :latest   "2014-12-05T00:00:00.000Z"}}}})
                                                 :unit  :year}
-                                  :value       {:value (du/->Timestamp "1980-01-01")
+                                  :value       {:value (du/->Timestamp #inst "1980-01-01")
                                                 :field {:field
                                                         (merge field-defaults
                                                                {:field-id           (id :users :last_login)
@@ -267,11 +268,11 @@
                                    :join-alias   "USERS__via__USER_ID"}]}
     :fk-field-ids #{(id :checkins :user_id)}
     :table-ids    #{(id :users)}}]
-  (let [expanded-form (ql/expand (wrap-inner-query (query checkins
-                                                     (ql/filter (ql/> (ql/datetime-field $user_id->users.last_login :year)
-                                                                      "1980-01-01")))))]
-    (mapv obj->map [expanded-form
-                    (resolve' expanded-form)])))
+  (qpt/with-h2-db-timezone
+    (let [expanded-form (ql/expand (wrap-inner-query (query checkins
+                                                       (ql/filter (ql/> (ql/datetime-field $user_id->users.last_login :year)
+                                                                        "1980-01-01")))))]
+      (mapv obj->map [expanded-form (resolve' expanded-form)]))))
 
 
 ;; sum aggregation w/ datetime breakout

test/metabase/query_processor_test.clj

@@ -9,7 +9,8 @@
              [driver :as driver]
              [util :as u]]
             [metabase.test.data :as data]
-            [metabase.test.data.datasets :as datasets]))
+            [metabase.test.data.datasets :as datasets]
+            [metabase.util.date :as du]))
 
 ;; make sure all the driver test extension namespaces are loaded <3 if this isn't done some things will get loaded at
 ;; the wrong time which can end up causing test databases to be created more than once, which fails
@@ -358,3 +359,13 @@
       driver/engine->driver
       driver/features
       (contains? :set-timezone)))
+
+(defmacro with-h2-db-timezone
+  "This macro is useful when testing pieces of the query pipeline (such as expand) where it's a basic unit test not
+  involving a database, but does need to parse dates"
+  [& body]
+  `(du/with-effective-timezone {:engine   :h2
+                                :timezone "UTC"
+                                :name     "mock_db"
+                                :id       1}
+    ~@body))

test/metabase/sync/analyze/classify_test.clj

@@ -22,7 +22,7 @@
                     Field [_ {:table_id            (u/get-id table)
                               :name                "Current fingerprint, already analzed"
                               :fingerprint_version Short/MAX_VALUE
-                              :last_analyzed       (du/->Timestamp "2017-08-09")}]
+                              :last_analyzed       (du/->Timestamp #inst "2017-08-09")}]
                     Field [_ {:table_id            (u/get-id table)
                               :name                "Old fingerprint, not analyzed"
                               :fingerprint_version (dec Short/MAX_VALUE)
@@ -30,6 +30,6 @@
                     Field [_ {:table_id            (u/get-id table)
                               :name                "Old fingerprint, already analzed"
                               :fingerprint_version (dec Short/MAX_VALUE)
-                              :last_analyzed       (du/->Timestamp "2017-08-09")}]]
+                              :last_analyzed       (du/->Timestamp #inst "2017-08-09")}]]
       (for [field (#'classify/fields-to-classify table)]
         (:name field)))))

test/metabase/sync/analyze/fingerprint_test.clj

@@ -226,7 +226,7 @@
                               :table_id            (data/id :venues)
                               :fingerprint         nil
                               :fingerprint_version 1
-                              :last_analyzed       (du/->Timestamp "2017-08-09")}]
+                              :last_analyzed       (du/->Timestamp #inst "2017-08-09")}]
     (with-redefs [i/latest-fingerprint-version 3
                   sample/sample-fields         (constantly [[field [1 2 3 4 5]]])
                   fingerprint/fingerprint      (constantly {:experimental {:fake-fingerprint? true}})]

test/metabase/sync/analyze_test.clj

@@ -15,7 +15,7 @@
             [toucan.util.test :as tt]))
 
 (def ^:private fake-analysis-completion-date
-  (du/->Timestamp "2017-08-01"))
+  (du/->Timestamp #inst "2017-08-01"))
 
 ;; Check that Fields do *not* get analyzed if they're not newly created and fingerprint version is current
 (expect
@@ -62,7 +62,7 @@
 (expect
   #{"Current fingerprint, not analyzed"}
   (with-redefs [i/latest-fingerprint-version Short/MAX_VALUE
-                du/new-sql-timestamp         (constantly (du/->Timestamp "1999-01-01"))]
+                du/new-sql-timestamp         (constantly (du/->Timestamp #inst "1999-01-01"))]
     (tt/with-temp* [Table [table]
                     Field [_ {:table_id            (u/get-id table)
                               :name                "Current fingerprint, not analyzed"
@@ -71,7 +71,7 @@
                     Field [_ {:table_id            (u/get-id table)
                               :name                "Current fingerprint, already analzed"
                               :fingerprint_version Short/MAX_VALUE
-                              :last_analyzed       (du/->Timestamp "2017-08-09")}]
+                              :last_analyzed       (du/->Timestamp #inst "2017-08-09")}]
                     Field [_ {:table_id            (u/get-id table)
                               :name                "Old fingerprint, not analyzed"
                               :fingerprint_version (dec Short/MAX_VALUE)
@@ -79,6 +79,6 @@
                     Field [_ {:table_id            (u/get-id table)
                               :name                "Old fingerprint, already analzed"
                               :fingerprint_version (dec Short/MAX_VALUE)
-                              :last_analyzed       (du/->Timestamp "2017-08-09")}]]
+                              :last_analyzed       (du/->Timestamp #inst "2017-08-09")}]]
       (#'analyze/update-fields-last-analyzed! table)
       (db/select-field :name Field :last_analyzed (du/new-sql-timestamp)))))