Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable the :set-timezone driver feature; allow to change the reporting timezone in the Localization settings #253

Merged
merged 17 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ on:
pull_request:

env:
METABASE_VERSION: v0.50.3
METABASE_VERSION: v0.50.6

jobs:
check-local-current-version:
Expand Down
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
# 1.50.1

### New features

* Enabled `:set-timezone` ([docs](https://www.metabase.com/docs/latest/configuring-metabase/localization#report-timezone)) Metabase feature in the driver. ([#200](https://github.com/ClickHouse/metabase-clickhouse-driver/issues/200))

### Other

* The driver now uses [`session_timezone` ClickHouse setting](https://clickhouse.com/docs/en/operations/settings/settings#session_timezone). This is necessary to support the `:set-timezone` feature; however, this setting [was introduced in 23.6](https://clickhouse.com/docs/en/whats-new/changelog/2023#236), which makes it the minimal required ClickHouse version to work with the driver.
slvrtrn marked this conversation as resolved.
Show resolved Hide resolved

# 1.50.0

After Metabase 0.50.0, a new naming convention exists for the driver's releases. The new one is intended to reflect the Metabase version the driver is supposed to run on. For example, the driver version 1.50.0 means that it should be used with Metabase v0.50.x or Metabase EE 1.50.x _only_, and it is _not guaranteed_ that this particular version of the driver can work with the previous or the following versions of Metabase.
Expand Down
15 changes: 14 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,23 @@

The driver aims to support the current stable and LTS releases (see [the related docs](https://clickhouse.com/docs/en/faq/operations/production#how-to-choose-between-clickhouse-releases)).

After 1.50.1:

| ClickHouse version | Supported? |
|-------------------------|-------------|
| 23.8+ | ✔ |
| 23.6 - 23.7 | Best effort |

1.50.0 and earlier:

| ClickHouse version | Supported? |
|-------------------------|-------------|
| 23.8+ | ✔ |
| 23.3-23.7 | Best effort |
| 23.3 - 23.7 | Best effort |

For [connection impersonation feature](https://www.metabase.com/learn/permissions/impersonation), the minimal required ClickHouse version is 24.4; otherwise, the feature is disabled by the driver.

The [CSV Uploads feature](https://www.metabase.com/docs/latest/databases/uploads) currently works only with ClickHouse Cloud (see [this issue](https://github.com/ClickHouse/metabase-clickhouse-driver/issues/230) for more details).

## Installation

Expand Down
64 changes: 58 additions & 6 deletions src/metabase/driver/clickhouse.clj
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,12 @@
[metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
[metabase.driver.sql.query-processor :as sql.qp]
[metabase.driver.sql.util :as sql.u]
[metabase.lib.metadata :as lib.metadata]
[metabase.query-processor.store :as qp.store]
[metabase.upload :as upload]
[metabase.util :as u]
[metabase.util.log :as log])
(:import [com.clickhouse.jdbc.internal ClickHouseStatementImpl]))
(:import [com.clickhouse.jdbc.internal ClickHouseStatementImpl]))

(set! *warn-on-reflection* true)

Expand All @@ -34,19 +37,25 @@

(doseq [[feature supported?] {:standard-deviation-aggregations true
:foreign-keys (not config/is-test?)
:set-timezone false
:convert-timezone false
:now true
:set-timezone true
:convert-timezone false ;; https://github.com/ClickHouse/metabase-clickhouse-driver/issues/254
:test/jvm-timezone-setting false
:schemas true
:datetime-diff true
:upload-with-auto-pk false
:window-functions/offset false}]

(defmethod driver/database-supports? [:clickhouse feature] [_driver _feature _db] supported?))

(def ^:private default-connection-details
{:user "default" :password "" :dbname "default" :host "localhost" :port "8123"})

;; (defn- get-report-timezone-id-safely
;; []
;; (try
;; (qp.timezone/report-timezone-id-if-supported)
;; (catch Throwable _e nil)))

(defn- connection-details->spec* [details]
(let [;; ensure defaults merge on top of nils
details (reduce-kv (fn [m k v] (assoc m k (or v (k default-connection-details))))
Expand Down Expand Up @@ -76,6 +85,50 @@
:custom_http_params "allow_experimental_analyzer=0"}
(sql-jdbc.common/handle-additional-options details :separator-style :url))))

(defmethod sql-jdbc.execute/do-with-connection-with-options :clickhouse
[driver db-or-id-or-spec {:keys [^String session-timezone write?] :as options} f]
(sql-jdbc.execute/do-with-resolved-connection
driver
db-or-id-or-spec
options
(fn [^java.sql.Connection conn]
(when-not (sql-jdbc.execute/recursive-connection?)
(let [settings (if session-timezone
(format "allow_experimental_analyzer=0,session_timezone=%s" session-timezone)
"allow_experimental_analyzer=0")]
(.setClientInfo conn com.clickhouse.jdbc.ClickHouseConnection/PROP_CUSTOM_HTTP_PARAMS settings))

(sql-jdbc.execute/set-best-transaction-level! driver conn)
(sql-jdbc.execute/set-time-zone-if-supported! driver conn session-timezone)
(when-let [db (cond
;; id?
(integer? db-or-id-or-spec)
(qp.store/with-metadata-provider db-or-id-or-spec
(lib.metadata/database (qp.store/metadata-provider)))
;; db?
(u/id db-or-id-or-spec) db-or-id-or-spec
;; otherwise it's a spec and we can't get the db
:else nil)]
(sql-jdbc.execute/set-role-if-supported! driver conn db))
(let [read-only? (not write?)]
(try
(log/trace (pr-str (list '.setReadOnly 'conn read-only?)))
(.setReadOnly conn read-only?)
(catch Throwable e
(log/debugf e "Error setting connection readOnly to %s" (pr-str read-only?)))))
(when-not write?
(try
(log/trace (pr-str '(.setAutoCommit conn true)))
(.setAutoCommit conn true)
(catch Throwable e
(log/debug e "Error enabling connection autoCommit"))))
(try
(log/trace (pr-str '(.setHoldability conn java.sql.ResultSet/CLOSE_CURSORS_AT_COMMIT)))
(.setHoldability conn java.sql.ResultSet/CLOSE_CURSORS_AT_COMMIT)
(catch Throwable e
(log/debug e "Error setting default holdability for connection"))))
(f conn))))

(def ^:private ^{:arglists '([db-details])} cloud?
"Returns true if the `db-details` are for a ClickHouse Cloud instance, and false otherwise. If it fails to connect
to the database, it throws a java.sql.SQLException."
Expand Down Expand Up @@ -158,8 +211,7 @@
::upload/boolean "Nullable(Boolean)"
::upload/date "Nullable(Date32)"
::upload/datetime "Nullable(DateTime64(3))"
;; FIXME: should be `Nullable(DateTime64(3))`
::upload/offset-datetime nil))
::upload/offset-datetime "Nullable(DateTime64(3))"))

(defmethod driver/table-name-length-limit :clickhouse
[_driver]
Expand Down
77 changes: 40 additions & 37 deletions src/metabase/driver/clickhouse_introspection.clj
Original file line number Diff line number Diff line change
Expand Up @@ -12,59 +12,59 @@

(def ^:private database-type->base-type
(sql-jdbc.sync/pattern-based-database-type->base-type
[[#"Array" :type/Array]
[#"Bool" :type/Boolean]
[#"DateTime64" :type/DateTime]
[#"DateTime" :type/DateTime]
[#"Date" :type/Date]
[#"Date32" :type/Date]
[#"Decimal" :type/Decimal]
[#"Enum8" :type/Text]
[#"Enum16" :type/Text]
[#"FixedString" :type/TextLike]
[#"Float32" :type/Float]
[#"Float64" :type/Float]
[#"Int8" :type/Integer]
[#"Int16" :type/Integer]
[#"Int32" :type/Integer]
[#"Int64" :type/BigInteger]
[#"IPv4" :type/IPAddress]
[#"IPv6" :type/IPAddress]
[#"Map" :type/Dictionary]
[#"String" :type/Text]
[#"Tuple" :type/*]
[#"UInt8" :type/Integer]
[#"UInt16" :type/Integer]
[#"UInt32" :type/Integer]
[#"UInt64" :type/BigInteger]
[#"UUID" :type/UUID]]))
[[#"array" :type/Array]
[#"bool" :type/Boolean]
[#"datetime64" :type/DateTime]
[#"datetime" :type/DateTime]
[#"date" :type/Date]
[#"date32" :type/Date]
[#"decimal" :type/Decimal]
[#"enum8" :type/Text]
[#"enum16" :type/Text]
[#"fixedstring" :type/TextLike]
[#"float32" :type/Float]
[#"float64" :type/Float]
[#"int8" :type/Integer]
[#"int16" :type/Integer]
[#"int32" :type/Integer]
[#"int64" :type/BigInteger]
[#"ipv4" :type/IPAddress]
[#"ipv6" :type/IPAddress]
[#"map" :type/Dictionary]
[#"string" :type/Text]
[#"tuple" :type/*]
[#"uint8" :type/Integer]
[#"uint16" :type/Integer]
[#"uint32" :type/Integer]
[#"uint64" :type/BigInteger]
[#"uuid" :type/UUID]]))

(defn- normalize-db-type
[db-type]
(cond
;; LowCardinality
(str/starts-with? db-type "LowCardinality")
(str/starts-with? db-type "lowcardinality")
(normalize-db-type (subs db-type 15 (- (count db-type) 1)))
;; Nullable
(str/starts-with? db-type "Nullable")
(str/starts-with? db-type "nullable")
(normalize-db-type (subs db-type 9 (- (count db-type) 1)))
;; DateTime64
(str/starts-with? db-type "DateTime64")
:type/DateTime ;; FIXME: should be type/DateTimeWithTZ (#200)
(str/starts-with? db-type "datetime64")
(if (> (count db-type) 13) :type/DateTimeWithTZ :type/DateTime)
;; DateTime
(str/starts-with? db-type "DateTime")
:type/DateTime ;; FIXME: should be type/DateTimeWithTZ (#200)
(str/starts-with? db-type "datetime")
(if (> (count db-type) 8) :type/DateTimeWithTZ :type/DateTime)
;; Enum*
(str/starts-with? db-type "Enum")
(str/starts-with? db-type "enum")
:type/Text
;; Map
(str/starts-with? db-type "Map")
(str/starts-with? db-type "map")
:type/Dictionary
;; Tuple
(str/starts-with? db-type "Tuple")
(str/starts-with? db-type "tuple")
:type/*
;; SimpleAggregateFunction
(str/starts-with? db-type "SimpleAggregateFunction")
(str/starts-with? db-type "simpleaggregatefunction")
(normalize-db-type (subs db-type (+ (str/index-of db-type ",") 2) (- (count db-type) 1)))
;; _
:else (or (database-type->base-type (keyword db-type)) :type/*)))
Expand All @@ -73,7 +73,10 @@
;; Nullable(DateTime) -> :type/DateTime, SimpleAggregateFunction(sum, Int64) -> :type/BigInteger, etc
(defmethod sql-jdbc.sync/database-type->base-type :clickhouse
[_ database-type]
(normalize-db-type (subs (str database-type) 1)))
(let [db-type (if (keyword? database-type)
(subs (str database-type) 1)
database-type)]
(normalize-db-type (u/lower-case-en db-type))))

(defmethod sql-jdbc.sync/excluded-schemas :clickhouse [_]
#{"system" "information_schema" "INFORMATION_SCHEMA"})
Expand Down
Loading