From 9741d739880e5435e2abec5e62cf49e306b71aad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cam=20Sa=C3=BCl?= <cammsaul@gmail.com>
Date: Mon, 2 Nov 2015 17:31:07 -0800
Subject: [PATCH] Fix setting timezone for Postgres
---
src/metabase/driver/generic_sql/native.clj | 40 ++++++++++---------
.../driver/generic_sql/query_processor.clj | 16 ++++----
src/metabase/driver/postgres.clj | 2 +-
3 files changed, 32 insertions(+), 26 deletions(-)
diff --git a/src/metabase/driver/generic_sql/native.clj b/src/metabase/driver/generic_sql/native.clj
index 2d97764d034..4a91b9d2bc0 100644
--- a/src/metabase/driver/generic_sql/native.clj
+++ b/src/metabase/driver/generic_sql/native.clj
@@ -25,26 +25,30 @@
{:keys [features set-timezone-sql]} (driver/engine->driver (:engine database))]
(jdbc/with-db-transaction [t-conn db-conn]
+ (let [^java.sql.Connection jdbc-connection (:connection t-conn)]
+ ;; Disable auto-commit for this transaction, that way shady queries are unable to modify the database
+ (.setAutoCommit jdbc-connection false)
+ (try
+ ;; Set the timezone if applicable
+ (when-let [timezone (driver/report-timezone)]
+ (when (and (seq timezone)
+ (contains? features :set-timezone))
+ (log/debug (u/format-color 'green "%s" set-timezone-sql))
+ (try (jdbc/db-do-prepared t-conn set-timezone-sql [timezone])
+ (catch Throwable e
+ (log/error (u/format-color 'red "Failed to set timezone: %s" (.getMessage e)))))))
- ;; Set the timezone if applicable. We do this *before* making the transaction read-only because some DBs
- ;; won't let you set the timezone on a read-only connection
- (when-let [timezone (driver/report-timezone)]
- (when (and (seq timezone)
- (contains? features :set-timezone))
- (log/debug (u/format-color 'green "%s" set-timezone-sql))
- (try (jdbc/db-do-prepared t-conn set-timezone-sql [timezone])
- (catch Throwable e
- (log/error (u/format-color 'red "Failed to set timezone: %s" (.getMessage e)))))))
+ ;; Now run the query itself
+ (log/debug (u/format-color 'green "%s" sql))
+ (let [[columns & [first-row :as rows]] (jdbc/query t-conn sql, :as-arrays? true)]
+ {:rows rows
+ :columns columns
+ :cols (for [[column first-value] (zipmap columns first-row)]
+ {:name column
+ :base_type (value->base-type first-value)})})
- ;; Now make the transaction read-only and run the query itself
- (.setReadOnly ^com.mchange.v2.c3p0.impl.NewProxyConnection (:connection t-conn) true)
- (log/debug (u/format-color 'green "%s" sql))
- (let [[columns & [first-row :as rows]] (jdbc/query t-conn sql, :as-arrays? true)]
- {:rows rows
- :columns columns
- :cols (for [[column first-value] (zipmap columns first-row)]
- {:name column
- :base_type (value->base-type first-value)})})))
+ ;; Rollback any changes made during this transaction just to be extra-double-sure JDBC doesn't try to commit them automatically for us
+ (finally (.rollback jdbc-connection))))))
(catch java.sql.SQLException e
(let [^String message (or (->> (.getMessage e) ; error message comes back like 'Column "ZID" not found; SQL statement: ... [error-code]' sometimes
(re-find #"^(.*);") ; the user already knows the SQL, and error code is meaningless
diff --git a/src/metabase/driver/generic_sql/query_processor.clj b/src/metabase/driver/generic_sql/query_processor.clj
index 47a2a36c076..be4133bd17a 100644
--- a/src/metabase/driver/generic_sql/query_processor.clj
+++ b/src/metabase/driver/generic_sql/query_processor.clj
@@ -1,17 +1,18 @@
(ns metabase.driver.generic-sql.query-processor
"The Query Processor is responsible for translating the Metabase Query Language into korma SQL forms."
(:require [clojure.core.match :refer [match]]
- [clojure.tools.logging :as log]
+ [clojure.java.jdbc :as jdbc]
[clojure.string :as s]
+ [clojure.tools.logging :as log]
(korma [core :as k]
[db :as kdb])
(korma.sql [fns :as kfns]
[utils :as utils])
[metabase.config :as config]
[metabase.driver :as driver]
- [metabase.driver.query-processor :as qp]
(metabase.driver.generic-sql [native :as native]
[util :refer :all])
+ [metabase.driver.query-processor :as qp]
[metabase.util :as u])
(:import java.sql.Timestamp
java.util.Date
@@ -235,11 +236,12 @@
(kdb/with-db (:db entity)
(if (and (seq timezone)
(contains? (:features driver) :set-timezone))
- (kdb/transaction
- (try (k/exec-raw [(:set-timezone-sql driver) [timezone]])
- (catch Throwable e
- (log/error (u/format-color 'red "Failed to set timezone: %s" (.getMessage e)))))
- (k/exec korma-query))
+ (try (kdb/transaction (k/exec-raw [(:set-timezone-sql driver) [timezone]])
+ (k/exec korma-query))
+ (catch Throwable e
+ (log/error (u/format-color 'red "Failed to set timezone:\n%s"
+ (with-out-str (jdbc/print-sql-exception-chain e))))
+ (k/exec korma-query)))
(k/exec korma-query))))
(catch java.sql.SQLException e
diff --git a/src/metabase/driver/postgres.clj b/src/metabase/driver/postgres.clj
index 714b4f46611..c17ebc39c09 100644
--- a/src/metabase/driver/postgres.clj
+++ b/src/metabase/driver/postgres.clj
@@ -196,7 +196,7 @@
:unix-timestamp->timestamp unix-timestamp->timestamp
:date date
:date-interval date-interval
- :set-timezone-sql "SET LOCAL timezone TO ?;"
+ :set-timezone-sql "UPDATE pg_settings SET setting = ? WHERE name ILIKE 'timezone';"
:driver-specific-sync-field! driver-specific-sync-field!
:humanize-connection-error-message humanize-connection-error-message}
sql-driver
--
GitLab