Merge pull request #8436 from metabase/bigquery-using-jvm-timezone

Add new flag for BigQuery to use JVM timezone [ci drivers]

.circleci/config.yml

+defaults: &defaults
+  working_directory: /home/circleci/metabase/metabase/
+  # This is just the circleci/clojure:lein-2.8.1-node-browsers image with make & yarn installed
+  docker:
+      - image: metabase/ci:java-8-lein-2.8.1-yarn-0.16.0
+
+version: 2.1
+jobs:
+  checkout:
+    <<: *defaults
+    steps:
+      - checkout
+      - persist_to_workspace:
+          root: /home/circleci/
+          paths:
+            - metabase/metabase
+
+  be-deps:
+    <<: *defaults
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - restore_cache:
+          keys:
+            - mb-deps-{{ checksum "yarn.lock" }}-{{ checksum "project.clj" }}
+            - mb-deps
+      - run: lein deps
+      - persist_to_workspace:
+          root: /home/circleci/
+          paths:
+            - .m2
+
+  be-tests:
+    <<: *defaults
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Run backend unit tests
+          command: lein with-profile +ci test
+          no_output_timeout: 5m
+
+  be-linter-eastwood:
+    <<: *defaults
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Run Eastwood linter
+          command: lein with-profile +ci eastwood
+          no_output_timeout: 5m
+
+  be-linter-docstring-checker:
+    <<: *defaults
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Run dockstring-checker
+          command: lein with-profile +ci docstring-checker
+          no_output_timeout: 5m
+
+  be-linter-bikeshed:
+    <<: *defaults
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Run dockstring-checker
+          command: lein with-profile +ci bikeshed
+          no_output_timeout: 5m
+
+  be-linter-reflection-warnings:
+    <<: *defaults
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Run dockstring-checker
+          command: ./bin/reflection-linter
+          no_output_timeout: 5m
+
+  be-tests-mysql:
+    working_directory: /home/circleci/metabase/metabase/
+    docker:
+      - image: metabase/ci:java-8-lein-2.8.1-yarn-0.16.0
+      - image: circleci/mysql:5.7.23
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Run backend unit tests (MySQL)
+          environment:
+            ENGINES: h2,mysql
+            MB_ENCRYPTION_SECRET_KEY: Orw0AAyzkO/kPTLJRxiyKoBHXa/d6ZcO+p+gpZO/wSQ=
+            MB_DB_TYPE: mysql
+            MB_DB_HOST: localhost
+            MB_DB_PORT: 3306
+            MB_DB_DBNAME: circle_test
+            MB_DB_USER: root
+            MB_MYSQL_TEST_USER: root
+          command: >
+            /home/circleci/metabase/metabase/.circleci/skip-driver-tests.sh mysql ||
+            lein with-profile +ci test
+          no_output_timeout: 5m
+
+  be-tests-postgres:
+    working_directory: /home/circleci/metabase/metabase/
+    docker:
+      - image: metabase/ci:java-8-lein-2.8.1-yarn-0.16.0
+      - image: circleci/postgres:9.6-alpine
+        environment:
+          POSTGRES_USER: circle_test
+          POSTGRES_DB: circle_test
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Run backend unit tests (Postgres)
+          environment:
+            ENGINES: h2,postgres
+            MB_DB_TYPE: postgres
+            MB_DB_PORT: 5432
+            MB_DB_HOST: localhost
+            MB_DB_DBNAME: circle_test
+            MB_DB_USER: circle_test
+            MB_POSTGRESQL_TEST_USER: circle_test
+          command: >
+            /home/circleci/metabase/metabase/.circleci/skip-driver-tests.sh postgres ||
+            lein with-profile +ci test
+          no_output_timeout: 5m
+
+  be-tests-sparksql:
+    working_directory: /home/circleci/metabase/metabase/
+    docker:
+      - image: metabase/ci:java-8-lein-2.8.1-yarn-0.16.0
+      - image: metabase/spark:2.1.1
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Make plugins dir
+          command: mkdir /home/circleci/metabase/metabase/plugins
+      - run:
+          name: Download SparkSQL deps JAR
+          command: wget --output-document=plugins/spark-deps.jar https://s3.amazonaws.com/sparksql-deps/metabase-sparksql-deps-1.2.1.spark2-standalone.jar
+      - run:
+          name: Wait for SparkSQL to be ready
+          command: while ! nc -z localhost 10000; do sleep 0.1; done
+          no_output_timeout: 5m
+      - run:
+          name: Run backend unit tests (SparkSQL)
+          environment:
+            ENGINES: h2,sparksql
+          command: >
+            /home/circleci/metabase/metabase/.circleci/skip-driver-tests.sh sparksql ||
+            lein with-profile +ci test
+          no_output_timeout: 5m
+
+  be-tests-mongo:
+    working_directory: /home/circleci/metabase/metabase/
+    docker:
+      - image: metabase/ci:java-8-lein-2.8.1-yarn-0.16.0
+      - image: circleci/mongo:3.2
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Run backend unit tests (MongoDB 3.2)
+          command: >
+            /home/circleci/metabase/metabase/.circleci/skip-driver-tests.sh mongo ||
+            lein with-profile +ci test
+          no_output_timeout: 5m
+
+  be-tests-vertica:
+    working_directory: /home/circleci/metabase/metabase/
+    docker:
+      - image: metabase/ci:java-8-lein-2.8.1-yarn-0.16.0
+      - image: sumitchawla/vertica
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Make plugins dir
+          command: mkdir /home/circleci/metabase/metabase/plugins
+      - run:
+          name: Download Vertica JAR
+          command: >
+            /home/circleci/metabase/metabase/.circleci/skip-driver-tests.sh vertica ||
+            wget --output-document=plugins/vertica-jdbc-7.1.2-0.jar $VERTICA_JDBC_JAR
+          no_output_timeout: 5m
+      - run:
+          name: Run backend unit tests (Vertica)
+          command: >
+            /home/circleci/metabase/metabase/.circleci/skip-driver-tests.sh vertica ||
+            lein with-profile +ci test
+          no_output_timeout: 5m
+
+  be-tests-sqlserver:
+    <<: *defaults
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Run backend unit tests (SQL Server)
+          command: >
+            /home/circleci/metabase/metabase/.circleci/skip-driver-tests.sh sqlserver ||
+            lein with-profile +ci test
+          no_output_timeout: 5m
+
+  be-tests-bigquery:
+    <<: *defaults
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Run backend unit tests (BigQuery)
+          environment:
+            ENGINES: h2,bigquery
+          command: >
+            /home/circleci/metabase/metabase/.circleci/skip-driver-tests.sh bigquery ||
+            lein with-profile +ci test
+          no_output_timeout: 5m
+
+  be-tests-sqlite:
+    <<: *defaults
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Run backend unit tests (SQLite)
+          environment:
+            ENGINES: h2,sqlite
+          command: >
+            /home/circleci/metabase/metabase/.circleci/skip-driver-tests.sh sqlite ||
+            lein with-profile +ci test
+
+  be-tests-druid:
+    <<: *defaults
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Run backend unit tests (Druid)
+          environment:
+            ENGINES: h2,druid
+          command: >
+            /home/circleci/metabase/metabase/.circleci/skip-driver-tests.sh druid ||
+            lein with-profile +ci test
+          no_output_timeout: 5m
+
+  be-tests-redshift:
+    <<: *defaults
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Run backend unit tests (Redshift)
+          environment:
+            ENGINES: h2,redshift
+          command: >
+            /home/circleci/metabase/metabase/.circleci/skip-driver-tests.sh redshift ||
+            lein with-profile +ci test
+          no_output_timeout: 5m
+
+
+  be-tests-presto:
+    working_directory: /home/circleci/metabase/metabase/
+    docker:
+      - image: metabase/ci:java-8-lein-2.8.1-yarn-0.16.0
+      - image: metabase/presto-mb-ci
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Run backend unit tests (Presto)
+          environment:
+            ENGINES: h2,presto
+            MB_PRESTO_TEST_HOST: localhost
+            MB_PRESTO_TEST_PORT: 8080
+          command: >
+            /home/circleci/metabase/metabase/.circleci/skip-driver-tests.sh presto ||
+            lein with-profile +ci test
+          no_output_timeout: 5m
+
+
+  be-tests-oracle:
+    <<: *defaults
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Make plugins dir
+          command: mkdir /home/circleci/metabase/metabase/plugins
+      - run:
+          name: Download Oracle JAR
+          command: >
+            /home/circleci/metabase/metabase/.circleci/skip-driver-tests.sh oracle ||
+            wget --output-document=/home/circleci/metabase/metabase/plugins/ojdbc7.jar $ORACLE_JDBC_JAR
+      - run:
+          name: Run backend unit tests (Oracle)
+          environment:
+            ENGINES: h2,oracle
+          command: >
+            /home/circleci/metabase/metabase/.circleci/skip-driver-tests.sh oracle ||
+            lein with-profile +ci test
+          no_output_timeout: 5m
+
+  fe-deps:
+    <<: *defaults
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - restore_cache:
+          keys:
+            - mb-deps-{{ checksum "yarn.lock" }}-{{ checksum "project.clj" }}
+            - mb-deps
+      - run:
+          name: Run yarn
+          command: SAUCE_CONNECT_DOWNLOAD_ON_INSTALL=true yarn
+          no_output_timeout: 5m
+      - persist_to_workspace:
+          root: /home/circleci/
+          paths:
+            - .yarn
+            - .yarn-cache
+            - metabase/metabase/node_modules
+
+  fe-tests-karma:
+    <<: *defaults
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Run frontend tests (karma)
+          command: yarn run test-karma
+          no_output_timeout: 5m
+
+  fe-tests-unit:
+    <<: *defaults
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Run frontend unit tests
+          command: yarn run test-unit
+          no_output_timeout: 5m
+
+  build-uberjar:
+    <<: *defaults
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Build uberjar
+          command: ./bin/build-for-test
+          no_output_timeout: 5m
+      - persist_to_workspace:
+          root: /home/circleci/
+          paths:
+            - metabase/metabase/resources/version.properties
+            - metabase/metabase/target/uberjar/metabase.jar
+
+  fe-tests-integrated:
+    <<: *defaults
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - run:
+          name: Run frontend integrated tests
+          command: yarn run test-integrated-no-build
+          no_output_timeout: 5m
+
+  cache-dependencies:
+    <<: *defaults
+    steps:
+      - attach_workspace:
+          at: /home/circleci/
+      - save_cache:
+          key: mb-deps-{{ checksum "yarn.lock" }}-{{ checksum "project.clj" }}
+          paths:
+            - /home/circleci/.m2
+            - /home/circleci/.yarn
+            - /home/circleci/.yarn-cache
+            - /home/circleci/metabase/metabase/node_modules
+            - /home/circleci/metabase/metabase/target/uberjar
+
+
+workflows:
+  version: 2
+  build:
+    jobs:
+      - checkout
+      - be-deps:
+          requires:
+            - checkout
+      - be-tests:
+          requires:
+            - be-deps
+      - be-linter-eastwood:
+          requires:
+            - be-deps
+      - be-linter-docstring-checker:
+          requires:
+            - be-deps
+      - be-linter-bikeshed:
+          requires:
+            - be-deps
+      - be-linter-reflection-warnings:
+          requires:
+            - be-deps
+      - be-tests-mysql:
+          requires:
+            - be-tests
+      - be-tests-postgres:
+          requires:
+            - be-tests
+      - be-tests-sparksql:
+          requires:
+            - be-tests
+      - be-tests-mongo:
+          requires:
+            - be-tests
+      - be-tests-sqlserver:
+          requires:
+            - be-tests
+      - be-tests-bigquery:
+          requires:
+            - be-tests
+      - be-tests-sqlite:
+          requires:
+            - be-tests
+      - be-tests-presto:
+          requires:
+            - be-tests
+      - be-tests-oracle:
+          requires:
+            - be-tests
+      - be-tests-druid:
+          requires:
+            - be-tests
+      - be-tests-redshift:
+          requires:
+            - be-tests
+      - be-tests-vertica:
+          requires:
+            - be-tests
+      - fe-deps:
+          requires:
+            - checkout
+      - fe-tests-karma:
+          requires:
+            - fe-deps
+      - fe-tests-unit:
+          requires:
+            - fe-deps
+      - build-uberjar:
+          requires:
+            - be-deps
+      - fe-tests-integrated:
+          requires:
+            - build-uberjar
+            - fe-deps
+      - cache-dependencies:
+          requires:
+            - be-deps
+            - fe-deps
+            - build-uberjar

.circleci/skip-driver-tests.sh

+#!/usr/bin/env bash
+
+set -eu
+
+COMMIT_MESSAGE=`git log -1 --oneline`
+
+! [[ "$CIRCLE_BRANCH" =~ ^master|release-.+$ ]] &&
+    ! [[ "$COMMIT_MESSAGE" == *"[ci all]"* ]] &&
+    ! [[ "$COMMIT_MESSAGE" == *"[ci drivers]"* ]] &&
+    ! [[ "$COMMIT_MESSAGE" == *"[ci $1]"* ]] &&
+    echo "Skipping driver tests: $1"

bin/build-for-test

@@ -29,11 +29,12 @@ check-uberjar-hash() {
 }
 
 build-uberjar-for-test() {
-  ./bin/build version
   echo "$VERSION_PROPERTY_NAME=$(source-hash)" >> resources/version.properties
   ./bin/build uberjar
 }
 
+./bin/build version
+
 if [ ! -f "target/uberjar/metabase.jar" ] || ! check-uberjar-hash; then
   echo "Building uberjar for testing"
   build-uberjar-for-test

circle.yml

-machine:
-  timezone:
-    America/Los_Angeles
-  java:
-    version:
-      openjdk8
-  node:
-    version: 8.9.0
-  services:
-    - docker
-dependencies:
-  override:
-    - lein deps
-    - npm install -g 'yarn@>=0.16.0'
-    # Forces the Sauce Connect binary to be downloaded during dependencies phase so it's cached
-    - SAUCE_CONNECT_DOWNLOAD_ON_INSTALL=true yarn
-    - mkdir plugins
-  cache_directories:
-    - "~/.yarn"
-    - "~/.yarn-cache"
-database:
-  post:
-    # MySQL doesn't load named timezone information automatically, you have to run this command to load it
-    # TODO - we only really need to do this step if we're testing against MySQL
-    - mysql_tzinfo_to_sql /usr/share/zoneinfo | mysql -u ubuntu mysql
-test:
-  override:
-    - ./bin/ci:
-        parallel: true
-deployment:
-  master:
-    branch: master
-    commands:
-      - ./bin/deploy-webhook $DEPLOY_WEBHOOK
-general:
-  artifacts:
-    - target/uberjar/metabase.jar
-    - screenshots
-experimental:
-  notify:
-    branches:
-      only:
-        - master
-        - /release-.*/

frontend/src/metabase/components/DatabaseDetailsForm.jsx

@@ -226,6 +226,33 @@ export default class DatabaseDetailsForm extends Component {
     } else if (isTunnelField(field) && !this.state.details["tunnel-enabled"]) {
       // don't show tunnel fields if tunnel isn't enabled
       return null;
+    } else if (field.name === "use-jvm-timezone") {
+      let on =
+        this.state.details["use-jvm-timezone"] == undefined
+          ? false
+          : this.state.details["use-jvm-timezone"];
+      return (
+        <FormField key={field.name} fieldName={field.name}>
+          <div className="flex align-center Form-offset">
+            <div className="Grid-cell--top">
+              <Toggle
+                value={on}
+                onChange={val =>
+                  this.onChange("use-jvm-timezone", val)
+                }
+              />
+            </div>
+            <div className="px2">
+              <h3
+              >{t`Use the Java Virtual Machine (JVM) timezone`}</h3>
+              <div style={{ maxWidth: "40rem" }} className="pt1">
+                {t`We suggest you leave this off unless you're doing manual timezone casting in
+                                many or most of your queries with this data.`}
+              </div>
+            </div>
+          </div>
+        </FormField>
+      );
     } else if (field.name === "let-user-control-scheduling") {
       let on =
         this.state.details["let-user-control-scheduling"] == undefined

frontend/test/__runner__/backend.js

@@ -54,6 +54,7 @@ export const BackendResource = createSharedResource("BackendResource", {
           "-Xverify:none", // Skip bytecode verification for the JAR so it launches faster
           "-Djava.awt.headless=true", // when running on macOS prevent little Java icon from popping up in Dock
           "--add-modules=java.xml.bind", // Tell Java 9 we want to use java.xml stuff
+          "-Duser.timezone=US/Pacific",
           "-jar",
           "target/uberjar/metabase.jar",
         ],

src/metabase/driver/bigquery.clj

@@ -178,34 +178,45 @@
                    (.setQuery query-string))]
      (google/execute (.query (.jobs client) project-id request)))))
 
-(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 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))
-
-(defn- parse-bigquery-time [time-string]
-  (->> time-string
-       (tformat/parse bigquery-time-format)
-       tcoerce/to-long
-       Time.))
-
-(defn- unparse-bigquery-time [coercible-to-dt]
+(def ^:private ^:dynamic *bigquery-timezone*
+  "BigQuery stores all of it's timestamps in UTC. That timezone can be changed via a SQL function invocation in a
+  native query, but that change in timezone is not conveyed through the BigQuery API. In most situations
+  `*bigquery-timezone*` will just be UTC. If the user is always changing the timezone via native SQL function
+  invocation, they can set their JVM TZ to the correct timezone, mark `use-jvm-timezone` to `true` and that will bind
+  this dynamic var to the JVM TZ rather than UTC"
+  time/utc)
+
+(defn- parse-timestamp-str [timezone]
+  (fn [s]
+    ;; Timestamp strings either come back as ISO-8601 strings or Unix timestamps in µs, e.g. "1.3963104E9"
+    (or
+     (du/->Timestamp s timezone)
+     ;; 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)))))
+
+(defn- bigquery-time-format [timezone]
+  (tformat/formatter "HH:mm:SS" timezone))
+
+(defn- parse-bigquery-time [timezone]
+  (fn [time-string]
+    (->> time-string
+         (tformat/parse (bigquery-time-format timezone))
+         tcoerce/to-long
+         Time.)))
+
+(defn- unparse-bigquery-time [timezone coercible-to-dt]
   (->> coercible-to-dt
        tcoerce/to-date-time
-       (tformat/unparse bigquery-time-format)))
+       (tformat/unparse (bigquery-time-format timezone))))
 
 (def ^:private type->parser
   "Functions that should be used to coerce string values in responses to the appropriate type for their column."
-  {"BOOLEAN"   #(Boolean/parseBoolean %)
-   "FLOAT"     #(Double/parseDouble %)
-   "INTEGER"   #(Long/parseLong %)
-   "RECORD"    identity
-   "STRING"    identity
+  {"BOOLEAN"   (constantly #(Boolean/parseBoolean %))
+   "FLOAT"     (constantly #(Double/parseDouble %))
+   "INTEGER"   (constantly #(Long/parseLong %))
+   "RECORD"    (constantly identity)
+   "STRING"    (constantly identity)
    "DATE"      parse-timestamp-str
    "DATETIME"  parse-timestamp-str
    "TIMESTAMP" parse-timestamp-str
@@ -225,8 +236,10 @@
        (post-process-native response (dec timeout-seconds)))
      ;; Otherwise the job *is* complete
      (let [^TableSchema schema (.getSchema response)
-           parsers             (for [^TableFieldSchema field (.getFields schema)]
-                                 (type->parser (.getType field)))
+           parsers             (doall
+                                (for [^TableFieldSchema field (.getFields schema)
+                                      :let [parser-fn (type->parser (.getType field))]]
+                                  (parser-fn *bigquery-timezone*)))
            columns             (for [column (table-schema->metabase-field-info schema)]
                                  (set/rename-keys column {:base-type :base_type}))]
        {:columns (map :name columns)
@@ -394,7 +407,7 @@
 (defmethod sqlqp/->honeysql [BigQueryDriver TimeValue]
   [driver {:keys [value]}]
   (->> value
-       unparse-bigquery-time
+       (unparse-bigquery-time *bigquery-timezone*)
        (sqlqp/->honeysql driver)
        hx/->time))
 
@@ -519,17 +532,24 @@
        :table-name table-name
        :mbql?      true})))
 
+(defn- effective-query-timezone [database]
+  (if-let [^java.util.TimeZone jvm-tz (and (get-in database [:details :use-jvm-timezone])
+                                           @du/jvm-timezone)]
+    (time/time-zone-for-id (.getID jvm-tz))
+    time/utc))
+
 (defn- execute-query [{database                                               :database
                        {sql :query, params :params, :keys [table-name mbql?]} :native
                        :as                                                    outer-query}]
-  (let [sql     (str "-- " (qputil/query->remark outer-query) "\n" (if (seq params)
-                                                                     (unprepare/unprepare (cons sql params))
-                                                                     sql))
-        results (process-native* database sql)
-        results (if mbql?
-                  (post-process-mbql table-name results)
-                  (update results :columns (partial map keyword)))]
-    (assoc results :annotate? mbql?)))
+  (binding [*bigquery-timezone* (effective-query-timezone database)]
+    (let [sql     (str "-- " (qputil/query->remark outer-query) "\n" (if (seq params)
+                                                                       (unprepare/unprepare (cons sql params))
+                                                                       sql))
+          results (process-native* database sql)
+          results (if mbql?
+                    (post-process-mbql table-name results)
+                    (update results :columns (partial map keyword)))]
+      (assoc results :annotate? mbql?))))
 
 
 ;; BigQuery doesn't return a timezone with it's time strings as it's always UTC, JodaTime parsing also defaults to UTC
@@ -579,8 +599,12 @@
                                                  {:name         "auth-code"
                                                   :display-name (tru "Auth Code")
                                                   :placeholder  "4/HSk-KtxkSzTt61j5zcbee2Rmm5JHkRFbL5gD5lgkXek"
-                                                  :required     true}])
-          :execute-query            (u/drop-first-arg execute-query)
+                                                  :required     true}
+                                                 {:name         "use-jvm-timezone"
+                                                  :display-name (tru "Use JVM Time Zone")
+                                                  :default      "false"
+                                                  :type         :boolean}])
+          :execute-query            (u/drop-first-arg #'execute-query)
           ;; Don't enable foreign keys when testing because BigQuery *doesn't* have a notion of foreign keys. Joins
           ;; are still allowed, which puts us in a weird position, however; people can manually specifiy "foreign key"
           ;; relationships in admin and everything should work correctly. Since we can't infer any "FK" relationships

test/metabase/driver/bigquery_test.clj

 (ns metabase.driver.bigquery-test
-  (:require [expectations :refer :all]
+  (:require [clj-time.core :as time]
+            [expectations :refer :all]
             [honeysql.core :as hsql]
             [metabase
              [driver :as driver]
              [query-processor :as qp]
-             [query-processor-test :as qptest]]
+             [query-processor-test :as qptest]
+             [util :as u]]
             [metabase.driver.bigquery :as bigquery]
             [metabase.models
+             [database :refer [Database]]
              [field :refer [Field]]
              [table :refer [Table]]]
             [metabase.query-processor.interface :as qpi]
@@ -14,7 +17,8 @@
             [metabase.test
              [data :as data]
              [util :as tu]]
-            [metabase.test.data.datasets :refer [expect-with-engine]]))
+            [metabase.test.data.datasets :refer [expect-with-engine]]
+            [toucan.util.test :as tt]))
 
 (def ^:private col-defaults
   {:remapped_to nil, :remapped_from nil})
@@ -168,3 +172,38 @@
 (expect
   ["SELECT `dataset.table`"]
   (hsql/format {:select [(#'bigquery/map->BigQueryIdentifier {:dataset-name "dataset", :table-name "table"})]}))
+
+(defn- native-timestamp-query [db-or-db-id timestamp-str timezone-str]
+  (-> (qp/process-query
+        {:native   {:query (format "select datetime(TIMESTAMP \"%s\", \"%s\")" timestamp-str timezone-str)
+                    :type  :native}
+         :database (u/get-id db-or-db-id)})
+      :data
+      :rows
+      ffirst))
+
+;; This query tests out the timezone handling of parsed dates. For this test a UTC date is returned, we should
+;; read/return it as UTC
+(expect-with-engine :bigquery
+  "2018-08-31T00:00:00.000Z"
+  (native-timestamp-query (data/id) "2018-08-31 00:00:00" "UTC"))
+
+;; This test includes a `use-jvm-timezone` flag of true that will assume that the date coming from BigQuery is already
+;; in the JVM's timezone. The test puts the JVM's timezone into America/Chicago an ensures that the correct date is
+;; compared
+(expect-with-engine :bigquery
+  "2018-08-31T00:00:00.000-05:00"
+  (tu/with-jvm-tz (time/time-zone-for-id "America/Chicago")
+    (tt/with-temp* [Database [db {:engine :bigquery
+                                  :details (assoc (:details (Database (data/id)))
+                                             :use-jvm-timezone true)}]]
+      (native-timestamp-query db "2018-08-31 00:00:00-05" "America/Chicago"))))
+
+;; Similar to the above test, but covers a positive offset
+(expect-with-engine :bigquery
+  "2018-08-31T00:00:00.000+07:00"
+  (tu/with-jvm-tz (time/time-zone-for-id "Asia/Jakarta")
+    (tt/with-temp* [Database [db {:engine :bigquery
+                                  :details (assoc (:details (Database (data/id)))
+                                             :use-jvm-timezone true)}]]
+      (native-timestamp-query db "2018-08-31 00:00:00+07" "Asia/Jakarta"))))

test/metabase/driver/mysql_test.clj

@@ -86,7 +86,7 @@
       (db->fields db))))
 
 (expect-with-engine :mysql
-  "America/Los_Angeles"
+  "UTC"
   (tu/db-timezone-id))
 
 (expect-with-engine :mysql

test/metabase/query_processor_test/timezones_test.clj

@@ -20,31 +20,11 @@
 
 (def ^:private mysql-driver (MySQLDriver.))
 
-(defn- fix-mysql-timestamps?
-  "Returns true if the database at `db-spec` needs to have it's timestamps fixed. See the `update-mysql-timestamps
-  comment for more information on why these are being fixed"
-  [db-spec]
-  (empty? (jdbc/query db-spec "select 1 from users where id=1 and last_login='2014-04-01 01:30:00'")))
-
-(defn- update-mysql-timestamps
-  "Unfortunately the timestamps we insert in this dataset are in UTC, but MySQL is inserting them as if they were in
-  pacific time. This means that they are rolling them forward 7 (or 8) hours. Instead of inserting 08:30 it's
-  inserting 15:30. This is wrong, rather than hack something together that weaves through all of the data loading
-  code, this function just fixes up the timestamps after the data is loaded using MySQL's `convert_tz` function"
-  []
-  (when (= :mysql *engine*)
-    (let [details (i/database->connection-details mysql-driver :db {:database-name "test-data-with-timezones"})
-          db-spec (sql/connection-details->spec mysql-driver details)]
-      (when (fix-mysql-timestamps? db-spec)
-        (jdbc/execute! db-spec "update users set last_login=convert_tz(last_login,'UTC','America/Los_Angeles')")))))
-
 (defn- call-with-timezones-db [f]
   ;; Does the database exist?
   (when-not (i/metabase-instance defs/test-data-with-timezones *engine*)
     ;; The database doesn't exist, so we need to create it
-    (data/get-or-create-database! defs/test-data-with-timezones)
-    ;; The db has been created but the timestamps are wrong on MySQL, fix them up
-    (update-mysql-timestamps))
+    (data/get-or-create-database! defs/test-data-with-timezones))
   ;; The database can now be used in tests
   (data/with-db (data/get-or-create-database! defs/test-data-with-timezones)
     (f)))

test/metabase/test/util.clj

@@ -36,6 +36,7 @@
             [metabase.test.data
              [dataset-definitions :as defs]
              [datasets :refer [*driver*]]]
+            [metabase.util.date :as du]
             [toucan.db :as db]
             [toucan.util.test :as test])
   (:import com.mchange.v2.c3p0.PooledDataSource
@@ -518,7 +519,8 @@
       (DateTimeZone/setDefault dtz)
       ;; We read the system property directly when formatting results, so this needs to be changed
       (System/setProperty "user.timezone" (.getID dtz))
-      (f)
+      (with-redefs [du/jvm-timezone (delay (.toTimeZone dtz))]
+        (f))
       (finally
         ;; We need to ensure we always put the timezones back the way
         ;; we found them as it will cause test failures