Convert-timezone for bigquery, snowflake, redshift (#26633)

* Bigquery

* Redshift

* Snowflake

* no convert-timezone for sparksql because it only has one timestamp data type

* redshift -- wrap type when adding interval

* don't override results-timezone-id in tests

* fix redshift ignore timezone when insert timestamp with time zone test data

modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk.clj

@@ -320,6 +320,10 @@
   [_driver _feat _db]
   true)
 
+(defmethod driver/database-supports? [:bigquery-cloud-sdk :convert-timezone]
+  [_driver _feat _db]
+  true)
+
 ;; BigQuery uses timezone operators and arguments on calls like extract() and timezone_trunc() rather than literally
 ;; using SET TIMEZONE, but we need to flag it as supporting set-timezone anyway so that reporting timezones are
 ;; returned and used, and tests expect the converted values.

modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk/query_processor.clj

@@ -10,6 +10,7 @@
             [metabase.driver.sql :as sql]
             [metabase.driver.sql.parameters.substitution :as sql.params.substitution]
             [metabase.driver.sql.query-processor :as sql.qp]
+            [metabase.driver.sql.util :as sql.u]
             [metabase.driver.sql.util.unprepare :as unprepare]
             [metabase.mbql.util :as mbql.u]
             [metabase.models.field :refer [Field]]
@@ -114,12 +115,12 @@
     (u.date/parse s timezone-id)))
 
 (defmethod parse-result-of-type "DATE"
-  [_ column-mode timezone-id v]
-  (parse-value column-mode v (partial parse-timestamp-str timezone-id)))
+  [_ column-mode _timezone-id v]
+  (parse-value column-mode v u.date/parse))
 
 (defmethod parse-result-of-type "DATETIME"
-  [_ column-mode timezone-id v]
-  (parse-value column-mode v (partial parse-timestamp-str timezone-id)))
+  [_ column-mode _timezone-id v]
+  (parse-value column-mode v u.date/parse))
 
 (defmethod parse-result-of-type "TIMESTAMP"
   [_ column-mode timezone-id v]
@@ -431,6 +432,18 @@
     [_ _ expr]
     (with-temporal-type (hsql/call bigquery-fn expr) :timestamp)))
 
+(defmethod sql.qp/->honeysql [:bigquery-cloud-sdk :convert-timezone]
+  [driver [_ arg target-timezone source-timezone]]
+  (let [datetime     (partial hsql/call :datetime)
+        hsql-form    (sql.qp/->honeysql driver arg)
+        timestamptz? (hx/is-of-type? hsql-form "timestamp")]
+    (sql.u/validate-convert-timezone-args timestamptz? target-timezone source-timezone)
+    (-> (if timestamptz?
+          hsql-form
+          (hsql/call :timestamp hsql-form (or source-timezone (qp.timezone/results-timezone-id))))
+        (datetime target-timezone)
+        (with-temporal-type :datetime))))
+
 (defmethod sql.qp/->float :bigquery-cloud-sdk
   [_ value]
   (hx/cast :float64 value))

modules/drivers/bigquery-cloud-sdk/test/metabase/driver/bigquery_cloud_sdk/params_test.clj

@@ -25,10 +25,10 @@
                                                                          :v         "16000000000000000000000000000000"}]
                             ;; LocalDate
                             [#t "2020-05-26"                            {:base-type :type/Date
-                                                                         :v         #t "2020-05-26T00:00Z[UTC]"}]
+                                                                         :v         #t "2020-05-26"}]
                             ;; LocaleDateTime
                             [#t "2020-05-26T17:06:00"                   {:base-type :type/DateTime
-                                                                         :v         #t "2020-05-26T17:06Z[UTC]"}]
+                                                                         :v         #t "2020-05-26T17:06"}]
                             ;; LocalTime
                             [#t "17:06:00"                              {:base-type :type/Time}]
                             ;; OffsetTime

modules/drivers/bigquery-cloud-sdk/test/metabase/driver/bigquery_cloud_sdk/query_processor_test.clj

@@ -68,9 +68,9 @@
                [1 2]
                [3.14159265359 0.5772156649]
                [1234M 5678M]
-               [#t "2018-01-01T00:00Z[UTC]" #t "2018-12-31T00:00Z[UTC]"]
+               [#t "2018-01-01" #t "2018-12-31"]
                [#t "12:34" #t "20:01:13.230"]
-               [#t "1957-05-17T03:35Z[UTC]" #t "2018-06-01T01:15:34.120Z[UTC]"]
+               [#t "1957-05-17T03:35" #t "2018-06-01T01:15:34.120"]
                [#t "2014-09-27T20:30:00.450Z[UTC]" #t "2020-09-27T14:57:00.450Z[UTC]"]
                []]]
              (mt/rows

modules/drivers/redshift/src/metabase/driver/redshift.clj

@@ -4,6 +4,7 @@
             [clojure.java.jdbc :as jdbc]
             [clojure.tools.logging :as log]
             [honeysql.core :as hsql]
+            [java-time :as t]
             [metabase.driver :as driver]
             [metabase.driver.common :as driver.common]
             [metabase.driver.sql-jdbc.common :as sql-jdbc.common]
@@ -84,11 +85,6 @@
   [_]
   :sunday)
 
-(defmethod driver/database-supports? [:redshift :convert-timezone]
-  [_driver _feat _db]
-  ;; TODO redshift could supports convert-timezone
-  false)
-
 ;;; +----------------------------------------------------------------------------------------------------------------+
 ;;; |                                           metabase.driver.sql impls                                            |
 ;;; +----------------------------------------------------------------------------------------------------------------+
@@ -116,7 +112,9 @@
 
 (defmethod sql.qp/add-interval-honeysql-form :redshift
   [_ hsql-form amount unit]
-  (hsql/call :dateadd (hx/literal unit) amount (hx/->timestamp hsql-form)))
+  (let [hsql-form (hx/->timestamp hsql-form)]
+    (-> (hsql/call :dateadd (hx/literal unit) amount hsql-form)
+        (hx/with-type-info (hx/type-info hsql-form)))))
 
 (defmethod sql.qp/unix-timestamp->honeysql [:redshift :seconds]
   [_ _ expr]
@@ -303,3 +301,7 @@
             #{}
             (sql-jdbc.describe-table/describe-table-fields-xf driver table)
             (sql-jdbc.describe-table/fallback-fields-metadata-from-select-query driver conn schema table-name))))))
+
+(defmethod sql-jdbc.execute/set-parameter [:redshift java.time.ZonedDateTime]
+  [driver ps i t]
+  (sql-jdbc.execute/set-parameter driver ps i (t/sql-timestamp (t/with-zone-same-instant t (t/zone-id "UTC")))))

modules/drivers/snowflake/src/metabase/driver/snowflake.clj

@@ -16,11 +16,13 @@
             [metabase.driver.sql-jdbc.execute.legacy-impl :as sql-jdbc.legacy]
             [metabase.driver.sql-jdbc.sync :as sql-jdbc.sync]
             [metabase.driver.sql.query-processor :as sql.qp]
+            [metabase.driver.sql.util :as sql.u]
             [metabase.driver.sql.util.unprepare :as unprepare]
             [metabase.driver.sync :as driver.s]
             [metabase.models.secret :as secret]
             [metabase.query-processor.error-type :as qp.error-type]
             [metabase.query-processor.store :as qp.store]
+            [metabase.query-processor.timezone :as qp.timezone]
             [metabase.query-processor.util.add-alias-info :as add]
             [metabase.util :as u]
             [metabase.util.date-2 :as u.date]
@@ -35,6 +37,10 @@
 
 (driver/register! :snowflake, :parent #{:sql-jdbc ::sql-jdbc.legacy/use-legacy-classes-for-read-and-set})
 
+(defmethod driver/supports? [:snowflake :convert-timezone]
+  [_driver _feature]
+  true)
+
 (defmethod driver/humanize-connection-error-message :snowflake
   [_ message]
   (log/spy :error (type message))
@@ -276,6 +282,18 @@
   [driver [_ value _unit]]
   (hx/->time (sql.qp/->honeysql driver value)))
 
+(defmethod sql.qp/->honeysql [:snowflake :convert-timezone]
+  [driver [_ arg target-timezone source-timezone]]
+  (let [hsql-form    (sql.qp/->honeysql driver arg)
+        timestamptz? (hx/is-of-type? hsql-form "timestamptz")]
+    (sql.u/validate-convert-timezone-args timestamptz? target-timezone source-timezone)
+    (-> (if timestamptz?
+          (hsql/call :convert_timezone target-timezone hsql-form)
+          (->> hsql-form
+               (hsql/call :convert_timezone (or source-timezone (qp.timezone/results-timezone-id)) target-timezone)
+               (hsql/call :to_timestamp_ntz)))
+        (hx/with-database-type-info "timestampntz"))))
+
 (defmethod driver/table-rows-seq :snowflake
   [driver database table]
   (sql-jdbc/query driver database {:select [:*]
@@ -365,7 +383,7 @@
 
 (defmethod unprepare/unprepare-value [:snowflake OffsetDateTime]
   [_ t]
-  (format "timestamp '%s %s %s'" (t/local-date t) (t/local-time t) (t/zone-offset t)))
+  (format "'%s %s %s'::timestamp_tz" (t/local-date t) (t/local-time t) (t/zone-offset t)))
 
 (defmethod unprepare/unprepare-value [:snowflake ZonedDateTime]
   [driver t]

modules/drivers/snowflake/test/metabase/driver/snowflake_test.clj

@@ -28,7 +28,7 @@
               #(str/replace % #"\s+" " ")
               ["DROP TABLE IF EXISTS \"v3_test-data\".\"PUBLIC\".\"users\";"
                "CREATE TABLE \"v3_test-data\".\"PUBLIC\".\"users\" (\"id\" INTEGER AUTOINCREMENT, \"name\" TEXT,
-                \"last_login\" TIMESTAMP_LTZ, \"password\" TEXT, PRIMARY KEY (\"id\")) ;"
+                \"last_login\" TIMESTAMP_NTZ, \"password\" TEXT, PRIMARY KEY (\"id\")) ;"
                "DROP TABLE IF EXISTS \"v3_test-data\".\"PUBLIC\".\"categories\";"
                "CREATE TABLE \"v3_test-data\".\"PUBLIC\".\"categories\" (\"id\" INTEGER AUTOINCREMENT, \"name\" TEXT NOT NULL,
                 PRIMARY KEY (\"id\")) ;"

modules/drivers/snowflake/test/metabase/test/data/snowflake.clj

@@ -19,7 +19,7 @@
 (doseq [[base-type sql-type] {:type/BigInteger     "BIGINT"
                               :type/Boolean        "BOOLEAN"
                               :type/Date           "DATE"
-                              :type/DateTime       "TIMESTAMP_LTZ"
+                              :type/DateTime       "TIMESTAMP_NTZ"
                               :type/DateTimeWithTZ "TIMESTAMP_TZ"
                               :type/Decimal        "DECIMAL"
                               :type/Float          "FLOAT"

test/metabase/driver/sql/parameters/substitute_test.clj

@@ -619,11 +619,6 @@
               (= driver/*driver* :vertica)
               "2018-04-17T00:00:00-07:00"
 
-              ;; TIMEZONE FIXME - bigquery doesn't support SET TIMEZONE, so the CAST to date below is going to make a UTC date,
-              ;; and then get converted back to reporting TZ (TODO: where?). But it's not clear if this behavior is actually relevant/an issue.
-              (= driver/*driver* :bigquery-cloud-sdk)
-              "2018-04-17T17:00:00-07:00"
-
               (qp.test/supports-report-timezone? driver/*driver*)
               "2018-04-18T00:00:00-07:00"
 

test/metabase/query_processor_test/alternative_date_test.clj

@@ -178,7 +178,9 @@
 (deftest iso-8601-text-fields
   (testing "text fields with semantic_type :type/ISO8601DateTimeString"
     (testing "return as dates"
-      (mt/test-drivers (disj (sql-jdbc.tu/sql-jdbc-drivers) :sqlite :oracle :sparksql)
+      (mt/test-drivers (-> (sql-jdbc.tu/sql-jdbc-drivers)
+                           (conj :bigquery-cloud-sdk)
+                           (disj :sqlite :oracle :sparksql))
         (is (= [[1 "foo" #t "2004-10-19T10:23:54" #t "2004-10-19" #t "10:23:54"]
                 [2 "bar" #t "2008-10-19T10:23:54" #t "2008-10-19" #t "10:23:54"]
                 [3 "baz" #t "2012-10-19T10:23:54" #t "2012-10-19" #t "10:23:54"]]
@@ -219,17 +221,6 @@
                               (assoc (mt/mbql-query times)
                                      :middleware {:format-rows? false}))))))))
 
-      (testing "bigquery adds UTC"
-        (mt/test-drivers #{:bigquery-cloud-sdk}
-          (is (= [[1 "foo" #t "2004-10-19T10:23:54Z[UTC]" #t "2004-10-19T00:00Z[UTC]" #t "10:23:54"]
-                  [2 "bar" #t "2008-10-19T10:23:54Z[UTC]" #t "2008-10-19T00:00Z[UTC]" #t "10:23:54"]
-                  [3 "baz" #t "2012-10-19T10:23:54Z[UTC]" #t "2012-10-19T00:00Z[UTC]" #t "10:23:54"]]
-                 ;; string-times dataset has three text fields, ts, d, t for timestamp, date, and time
-                 (mt/rows (mt/dataset string-times
-                            (qp/process-query
-                              (assoc (mt/mbql-query times)
-                                     :middleware {:format-rows? false}))))))))
-
       (testing "mongo only supports datetime"
         (mt/test-drivers #{:mongo}
           (mt/dataset string-times
@@ -330,11 +321,11 @@
              [[1 "foo" (.toInstant #t "2019-04-21T16:43:00Z")]
               [2 "bar" (.toInstant #t "2020-04-21T16:43:00Z")]
               [3 "baz" (.toInstant #t "2021-04-21T16:43:00Z")]]
-             (:h2 :mysql :sqlserver)
+             (:h2 :mysql :sqlserver :bigquery-cloud-sdk)
              [[1 "foo" #t "2019-04-21T16:43"]
               [2 "bar" #t "2020-04-21T16:43"]
               [3 "baz" #t "2021-04-21T16:43"]]
-             (:bigquery-cloud-sdk :redshift :presto)
+             (:redshift :presto)
              [[1 "foo" #t "2019-04-21T16:43Z[UTC]"]
               [2 "bar" #t "2020-04-21T16:43Z[UTC]"]
               [3 "baz" #t "2021-04-21T16:43Z[UTC]"]]

test/metabase/query_processor_test/date_time_zone_functions_test.clj

@@ -420,12 +420,13 @@
                                   [:convert-timezone [:field (mt/id :times :dt_tz) nil] "Asia/Tokyo"]))))))))
 
         (testing "with literal datetime"
-          (is (= "2022-10-03T14:10:20+07:00"
-                 (->> (mt/run-mbql-query times
-                                         {:expressions {"expr" [:convert-timezone "2022-10-03T07:10:20" "Asia/Saigon" "UTC"]}
-                                          :fields      [[:expression "expr"]]})
-                      mt/rows
-                      ffirst))))))))
+          (mt/with-report-timezone-id "UTC"
+            (is (= "2022-10-03T14:10:20+07:00"
+                   (->> (mt/run-mbql-query times
+                                           {:expressions {"expr" [:convert-timezone "2022-10-03T07:10:20" "Asia/Saigon" "UTC"]}
+                                            :fields      [[:expression "expr"]]})
+                        mt/rows
+                        ffirst)))))))))
 
 (deftest nested-convert-timezone-test
   (mt/test-drivers (mt/normal-drivers-with-feature :convert-timezone)