From bbf79d53fd94eee6d60cd945133fa8149a56a785 Mon Sep 17 00:00:00 2001 From: lbrdnk <lbrdnk@users.noreply.github.com> Date: Thu, 18 Jan 2024 21:49:39 +0100 Subject: [PATCH] Add :timezone key to database metadata (#36413) * Add timezone to database metadata * Use UTC as _user timezone_ for snowflake driver * Use user timezone as db-default-timezone * Update oracles unprepare-value to handle Z UTC * Update `driver/db-default-timezone :h2` to use jvm timezone * Update containers timezone to match instance timezone and tests * Update modules/drivers/oracle/src/metabase/driver/oracle.clj Co-authored-by: metamben <103100869+metamben@users.noreply.github.com> * Make unprepare symmetric for offset and zoned datetime --------- Co-authored-by: metamben <103100869+metamben@users.noreply.github.com> --- .../actions/e2e-prepare-containers/action.yml | 4 +- .../actions/actions-on-dashboards.cy.spec.js | 43 ++++++------------- e2e/test/scenarios/docker-compose.yml | 4 ++ .../oracle/src/metabase/driver/oracle.clj | 10 ++--- .../src/metabase/driver/snowflake.clj | 4 +- .../test/metabase/test/data/snowflake.clj | 14 ++++++ src/metabase/driver/h2.clj | 13 ++---- src/metabase/lib/metadata/jvm.clj | 2 +- 8 files changed, 44 insertions(+), 50 deletions(-) diff --git a/.github/actions/e2e-prepare-containers/action.yml b/.github/actions/e2e-prepare-containers/action.yml index d00f29e227d..fe5fefe72e0 100644 --- a/.github/actions/e2e-prepare-containers/action.yml +++ b/.github/actions/e2e-prepare-containers/action.yml @@ -66,14 +66,14 @@ runs: if ${{ inputs.postgres }}; then echo -e "${Y}Starting postgres container..." && - docker run -d -p 5404:5432 metabase/qa-databases:postgres-sample-12 && + docker run -d -p 5404:5432 -e TZ='US/Pacific' metabase/qa-databases:postgres-sample-12 && while ! nc -z localhost 5404; do sleep 1; done && echo -e "${G}Postgres is up and running!" fi if ${{ inputs.mysql }}; then echo -e "${Y}Starting mysql container..." && - docker run -d -p 3304:3306 metabase/qa-databases:mysql-sample-8 && + docker run -d -p 3304:3306 -e TZ='US/Pacific' metabase/qa-databases:mysql-sample-8 && while ! nc -z localhost 3304; do sleep 1; done && echo -e "${G}MySQL is up and running!" fi diff --git a/e2e/test/scenarios/actions/actions-on-dashboards.cy.spec.js b/e2e/test/scenarios/actions/actions-on-dashboards.cy.spec.js index 87007c77f7a..af7ba4dcedf 100644 --- a/e2e/test/scenarios/actions/actions-on-dashboards.cy.spec.js +++ b/e2e/test/scenarios/actions/actions-on-dashboards.cy.spec.js @@ -747,39 +747,20 @@ const MODEL_NAME = "Test Action Model"; newValue: newTime, }); - // only postgres has timezone-aware columns - // the instance is in US/Pacific so it's -8 hours - if (dialect === "postgres") { - changeValue({ - fieldName: "DatetimeTZ", - fieldType: "datetime-local", - oldValue: "2020-01-01T00:35:55", - newValue: newTime, - }); + changeValue({ + fieldName: "DatetimeTZ", + fieldType: "datetime-local", + oldValue: oldRow.datetimeTZ.replace(" ", "T"), + newValue: newTime, + }); - changeValue({ - fieldName: "TimestampTZ", - fieldType: "datetime-local", - oldValue: "2020-01-01T00:35:55", - newValue: newTime, - }); - } - - if (dialect === "mysql") { - changeValue({ - fieldName: "DatetimeTZ", - fieldType: "datetime-local", - oldValue: oldRow.datetimeTZ.replace(" ", "T"), - newValue: newTime, - }); + changeValue({ + fieldName: "TimestampTZ", + fieldType: "datetime-local", + oldValue: oldRow.timestampTZ.replace(" ", "T"), + newValue: newTime, + }); - changeValue({ - fieldName: "TimestampTZ", - fieldType: "datetime-local", - oldValue: oldRow.timestampTZ.replace(" ", "T"), - newValue: newTime, - }); - } cy.button("Update").click(); }); diff --git a/e2e/test/scenarios/docker-compose.yml b/e2e/test/scenarios/docker-compose.yml index ed9729fa994..98de38f11ab 100644 --- a/e2e/test/scenarios/docker-compose.yml +++ b/e2e/test/scenarios/docker-compose.yml @@ -2,6 +2,8 @@ version: '3.9' services: postgres-sample: + environment: + - TZ=US/Pacific image: metabase/qa-databases:postgres-sample-12 ports: - "5404:5432" @@ -10,6 +12,8 @@ services: ports: - 27004:27017 mysql-sample: + environment: + - TZ=US/Pacific image: metabase/qa-databases:mysql-sample-8 ports: - 3304:3306 diff --git a/modules/drivers/oracle/src/metabase/driver/oracle.clj b/modules/drivers/oracle/src/metabase/driver/oracle.clj index 3a9191c54a4..1af1a51028c 100644 --- a/modules/drivers/oracle/src/metabase/driver/oracle.clj +++ b/modules/drivers/oracle/src/metabase/driver/oracle.clj @@ -620,14 +620,14 @@ (defmethod unprepare/unprepare-value [:oracle OffsetDateTime] [_ t] - (let [s (-> (t/format "yyyy-MM-dd HH:mm:ss.SSS ZZZZZ" t) - ;; Oracle doesn't like `Z` to mean UTC - (str/replace #"Z$" "UTC"))] - (format "timestamp '%s'" s))) + ;; Oracle doesn't like `Z` to mean UTC + (format "timestamp '%s'" (-> (t/format "yyyy-MM-dd HH:mm:ss.SSS ZZZZZ" t) + (str/replace #" Z$" " UTC")))) (defmethod unprepare/unprepare-value [:oracle ZonedDateTime] [_ t] - (format "timestamp '%s'" (t/format "yyyy-MM-dd HH:mm:ss.SSS VV" t))) + (format "timestamp '%s'" (-> (t/format "yyyy-MM-dd HH:mm:ss.SSS VV" t) + (str/replace #" Z$" " UTC")))) (defmethod unprepare/unprepare-value [:oracle Instant] [driver t] diff --git a/modules/drivers/snowflake/src/metabase/driver/snowflake.clj b/modules/drivers/snowflake/src/metabase/driver/snowflake.clj index 369d8142d36..9d66493ec3b 100644 --- a/modules/drivers/snowflake/src/metabase/driver/snowflake.clj +++ b/modules/drivers/snowflake/src/metabase/driver/snowflake.clj @@ -526,10 +526,10 @@ (sql-jdbc.execute/do-with-connection-with-options driver database nil (fn [^java.sql.Connection conn] - (with-open [stmt (.prepareStatement conn "show parameters like 'TIMEZONE';") + (with-open [stmt (.prepareStatement conn "show parameters like 'TIMEZONE' in user;") rset (.executeQuery stmt)] (when (.next rset) - (.getString rset "default")))))) + (.getString rset "value")))))) (defmethod sql-jdbc.sync/excluded-schemas :snowflake [_] diff --git a/modules/drivers/snowflake/test/metabase/test/data/snowflake.clj b/modules/drivers/snowflake/test/metabase/test/data/snowflake.clj index 2ad911a37c8..b5c40940db6 100644 --- a/modules/drivers/snowflake/test/metabase/test/data/snowflake.clj +++ b/modules/drivers/snowflake/test/metabase/test/data/snowflake.clj @@ -144,12 +144,26 @@ (delete-old-datasets!) (reset! deleted-old-datasets? true))))) +(defn- set-current-user-timezone! + [timezone] + (sql-jdbc.execute/do-with-connection-with-options + :snowflake + (no-db-connection-spec) + {:write? true} + (fn [^java.sql.Connection conn] + (with-open [stmt (.createStatement conn)] + (.execute stmt (format "ALTER USER SET TIMEZONE = '%s';" timezone)))))) + (defmethod tx/create-db! :snowflake [driver db-def & options] ;; qualify the DB name with the unique prefix (let [db-def (update db-def :database-name qualified-db-name)] ;; clean up any old datasets that should be deleted (delete-old-datsets-if-needed!) + ;; Snowflake by default uses America/Los_Angeles timezone. See https://docs.snowflake.com/en/sql-reference/parameters#timezone. + ;; We expect UTC in tests. Hence fixing [[metabase.query-processor.timezone/database-timezone-id]] (PR #36413) + ;; produced lot of failures. Following expression addresses that, setting timezone for the test user. + (set-current-user-timezone! "UTC") ;; now call the default impl for SQL JDBC drivers (apply (get-method tx/create-db! :sql-jdbc/test-extensions) driver db-def options))) diff --git a/src/metabase/driver/h2.clj b/src/metabase/driver/h2.clj index 49304f48ea1..5eb10468578 100644 --- a/src/metabase/driver/h2.clj +++ b/src/metabase/driver/h2.clj @@ -312,15 +312,10 @@ message)) (defmethod driver/db-default-timezone :h2 - [driver database] - (sql-jdbc.execute/do-with-connection-with-options - driver database nil - (fn [^java.sql.Connection conn] - (with-open [stmt (.prepareStatement conn "select current_timestamp();") - rset (.executeQuery stmt)] - (when (.next rset) - (when-let [zoned-date-time (.getObject rset 1 java.time.ZonedDateTime)] - (t/zone-id zoned-date-time))))))) + [_driver _database] + ;; Based on this answer https://stackoverflow.com/a/18883531 and further experiments, h2 uses timezone of the jvm + ;; where the driver is loaded. + (System/getProperty "user.timezone")) ;;; +----------------------------------------------------------------------------------------------------------------+ diff --git a/src/metabase/lib/metadata/jvm.clj b/src/metabase/lib/metadata/jvm.clj index 7f964ed16ac..6ceeb40c2b0 100644 --- a/src/metabase/lib/metadata/jvm.clj +++ b/src/metabase/lib/metadata/jvm.clj @@ -65,7 +65,7 @@ #_resolved-query clojure.lang.IPersistentMap] [query-type model parsed-args honeysql] (merge (next-method query-type model parsed-args honeysql) - {:select [:id :engine :name :dbms_version :settings :is_audit :details]})) + {:select [:id :engine :name :dbms_version :settings :is_audit :details :timezone]})) (t2/define-after-select :metadata/database [database] -- GitLab