diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 6bc8e1b..acb5ad4 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -10,7 +10,7 @@ on: pull_request: env: - METABASE_VERSION: v0.50.3 + METABASE_VERSION: v0.50.6 jobs: check-local-current-version: diff --git a/CHANGELOG.md b/CHANGELOG.md index ba44b6e..0b12efd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. + # 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. diff --git a/README.md b/README.md index 1e489bc..3105e5f 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/src/metabase/driver/clickhouse.clj b/src/metabase/driver/clickhouse.clj index d116675..20be397 100644 --- a/src/metabase/driver/clickhouse.clj +++ b/src/metabase/driver/clickhouse.clj @@ -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) @@ -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)))) @@ -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." @@ -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] diff --git a/src/metabase/driver/clickhouse_introspection.clj b/src/metabase/driver/clickhouse_introspection.clj index 4d6e9fd..888a2e4 100644 --- a/src/metabase/driver/clickhouse_introspection.clj +++ b/src/metabase/driver/clickhouse_introspection.clj @@ -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/*))) @@ -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"}) diff --git a/src/metabase/driver/clickhouse_qp.clj b/src/metabase/driver/clickhouse_qp.clj index 85f3412..f9cd1a7 100644 --- a/src/metabase/driver/clickhouse_qp.clj +++ b/src/metabase/driver/clickhouse_qp.clj @@ -10,8 +10,8 @@ [metabase.driver.sql.query-processor :as sql.qp :refer [add-interval-honeysql-form]] [metabase.driver.sql.util.unprepare :as unprepare] [metabase.legacy-mbql.util :as mbql.u] + [metabase.query-processor.timezone :as qp.timezone] [metabase.util :as u] - [metabase.util.date-2 :as u.date] [metabase.util.honey-sql-2 :as h2x] [metabase.util.log :as log]) (:import [com.clickhouse.data.value ClickHouseArrayValue] @@ -29,86 +29,147 @@ (defmethod sql.qp/quote-style :clickhouse [_] :mysql) -(defmethod sql.qp/date [:clickhouse :day-of-week] - [_ _ expr] - ;; a tick in the function name prevents HSQL2 to make the function call UPPERCASE - ;; https://cljdoc.org/d/com.github.seancorfield/honeysql/2.4.1011/doc/getting-started/other-databases#clickhouse - (sql.qp/adjust-day-of-week :clickhouse [:'dayOfWeek expr])) +;; without try, there might be test failures when QP is not yet initialized +;; e.g., when a test is preparing the dataset +(defn- get-report-timezone-id-safely + [] + (try + (qp.timezone/report-timezone-id-if-supported) + (catch Throwable _e nil))) + +;; datetime('europe/amsterdam') -> europe/amsterdam +(defn- extract-datetime-timezone + [db-type] + (when (and db-type (string? db-type)) + (cond + ;; e.g. DateTime64(3, 'Europe/Amsterdam') + (str/starts-with? db-type "datetime64") + (if (> (count db-type) 17) (subs db-type 15 (- (count db-type) 2)) nil) + ;; e.g. DateTime('Europe/Amsterdam') + (str/starts-with? db-type "datetime") + (if (> (count db-type) 12) (subs db-type 10 (- (count db-type) 2)) nil) + ;; _ + :else nil))) + +(defn- remove-low-cardinality-and-nullable + [db-type] + (when (and db-type (string? db-type)) + (let [without-low-car (if (str/starts-with? db-type "lowcardinality(") + (subs db-type 15 (- (count db-type) 1)) + db-type) + without-nullable (if (str/starts-with? without-low-car "nullable(") + (subs without-low-car 9 (- (count without-low-car) 1)) + without-low-car)] + without-nullable))) + +(defn- in-report-timezone + [expr] + (let [report-timezone (get-report-timezone-id-safely) + lower (u/lower-case-en (h2x/database-type expr)) + db-type (remove-low-cardinality-and-nullable lower)] + (if (and report-timezone (string? db-type) (str/starts-with? db-type "datetime")) + (let [timezone (extract-datetime-timezone db-type)] + (if (not (= timezone (u/lower-case-en report-timezone))) + [:'toTimeZone expr (h2x/literal report-timezone)] + expr)) + expr))) (defmethod sql.qp/date [:clickhouse :default] [_ _ expr] expr) -(defmethod sql.qp/date [:clickhouse :minute] +;;; ------------------------------------------------------------------------------------ +;;; Extract functions +;;; ------------------------------------------------------------------------------------ + +(defn- date-extract + [ch-fn expr db-type] + (-> [ch-fn (in-report-timezone expr)] + (h2x/with-database-type-info db-type))) + +(defmethod sql.qp/date [:clickhouse :day-of-week] [_ _ expr] - [:'toStartOfMinute expr]) + ;; a tick in the function name prevents HSQL2 to make the function call UPPERCASE + ;; https://cljdoc.org/d/com.github.seancorfield/honeysql/2.4.1011/doc/getting-started/other-databases#clickhouse + (sql.qp/adjust-day-of-week + :clickhouse (date-extract :'toDayOfWeek expr "uint8"))) -(defmethod sql.qp/date [:clickhouse :minute-of-hour] +(defmethod sql.qp/date [:clickhouse :month-of-year] [_ _ expr] - [:'toMinute expr]) + (date-extract :'toMonth expr "uint8")) -(defmethod sql.qp/date [:clickhouse :hour] +(defmethod sql.qp/date [:clickhouse :minute-of-hour] [_ _ expr] - [:'toStartOfHour expr]) + (date-extract :'toMinute expr "uint8")) (defmethod sql.qp/date [:clickhouse :hour-of-day] [_ _ expr] - [:'toHour expr]) + (date-extract :'toHour expr "uint8")) (defmethod sql.qp/date [:clickhouse :day-of-month] [_ _ expr] - [:'toDayOfMonth expr]) - -(defn- to-start-of-week - [expr] - [:'toMonday expr]) - -(defn- to-start-of-year - [expr] - [:'toStartOfYear expr]) - -(defn- to-relative-day-num - [expr] - [:'toRelativeDayNum expr]) + (date-extract :'toDayOfMonth expr "uint8")) (defmethod sql.qp/date [:clickhouse :day-of-year] [_ _ expr] - (h2x/+ - (h2x/- (to-relative-day-num expr) - (to-relative-day-num (to-start-of-year expr))) - 1)) + (date-extract :'toDayOfYear expr "uint16")) (defmethod sql.qp/date [:clickhouse :week-of-year-iso] [_ _ expr] - [:'toISOWeek expr]) + (date-extract :'toISOWeek expr "uint8")) -(defmethod sql.qp/date [:clickhouse :month] +(defmethod sql.qp/date [:clickhouse :quarter-of-year] [_ _ expr] - [:'toStartOfMonth expr]) + (date-extract :'toQuarter expr "uint8")) -(defmethod sql.qp/date [:clickhouse :month-of-year] +(defmethod sql.qp/date [:clickhouse :year-of-era] [_ _ expr] - [:'toMonth expr]) + (date-extract :'toYear expr "uint16")) -(defmethod sql.qp/date [:clickhouse :quarter-of-year] +;;; ------------------------------------------------------------------------------------ +;;; Truncate functions +;;; ------------------------------------------------------------------------------------ + +(defn- date-trunc + [ch-fn expr] + (-> [ch-fn (in-report-timezone expr)] + (h2x/with-database-type-info (h2x/database-type expr)))) + +(defn- to-start-of-week + [expr] + (date-trunc :'toMonday expr)) + +(defmethod sql.qp/date [:clickhouse :minute] [_ _ expr] - [:'toQuarter expr]) + (date-trunc :'toStartOfMinute expr)) -(defmethod sql.qp/date [:clickhouse :year] +(defmethod sql.qp/date [:clickhouse :hour] [_ _ expr] - [:'toStartOfYear expr]) + (date-trunc :'toStartOfHour expr)) (defmethod sql.qp/date [:clickhouse :day] [_ _ expr] - [:'toDate expr]) + (date-trunc :'toStartOfDay expr)) (defmethod sql.qp/date [:clickhouse :week] [driver _ expr] (sql.qp/adjust-start-of-week driver to-start-of-week expr)) +(defmethod sql.qp/date [:clickhouse :month] + [_ _ expr] + (date-trunc :'toStartOfMonth expr)) + (defmethod sql.qp/date [:clickhouse :quarter] [_ _ expr] - [:'toStartOfQuarter expr]) + (date-trunc :'toStartOfQuarter expr)) + +(defmethod sql.qp/date [:clickhouse :year] + [_ _ expr] + (date-trunc :'toStartOfYear expr)) + +;;; ------------------------------------------------------------------------------------ +;;; Unix timestamps functions +;;; ------------------------------------------------------------------------------------ (defmethod sql.qp/unix-timestamp->honeysql [:clickhouse :seconds] [_ _ expr] @@ -116,7 +177,49 @@ (defmethod sql.qp/unix-timestamp->honeysql [:clickhouse :milliseconds] [_ _ expr] - [:'toDateTime64 (h2x// expr 1000) 3]) + (let [report-timezone (get-report-timezone-id-safely) + inner-expr (h2x// expr 1000)] + (if report-timezone + [:'toDateTime64 inner-expr 3 report-timezone] + [:'toDateTime64 inner-expr 3]))) + +(defmethod sql.qp/unix-timestamp->honeysql [:clickhouse :microseconds] + [_ _ expr] + (let [report-timezone (get-report-timezone-id-safely) + inner-expr [:'toInt64 (h2x// expr 1000)]] + (if report-timezone + [:'fromUnixTimestamp64Milli inner-expr report-timezone] + [:'fromUnixTimestamp64Milli inner-expr]))) + +;;; ------------------------------------------------------------------------------------ +;;; HoneySQL forms +;;; ------------------------------------------------------------------------------------ + +;; Commented out until we enable :convert-timezone feature - this implementation is still not correct +;; There are several failing assertions in metabase.query-processor-test.date-time-zone-functions-test +;; See also: https://github.com/ClickHouse/metabase-clickhouse-driver/issues/254 +#_(defmethod sql.qp/->honeysql [:clickhouse :convert-timezone] + [driver [_ arg target-timezone source-timezone]] + (let [expr (sql.qp/->honeysql driver (cond-> arg (string? arg) u.date/parse)) + with-tz-info? (h2x/is-of-type? expr #"(?:nullable\(|lowcardinality\()?(datetime64\(\d, {0,1}'.*|datetime\(.*)") + _ (sql.u/validate-convert-timezone-args with-tz-info? target-timezone source-timezone) + inner (if (not with-tz-info?) + [:'plus + expr + [:'toIntervalSecond + [:'minus + [:'timeZoneOffset [:'now target-timezone]] + [:'timeZoneOffset [:'now source-timezone]]]]] + [:'toTimeZone expr target-timezone])] + inner)) + +(defmethod sql.qp/current-datetime-honeysql-form :clickhouse + [_] + (let [report-timezone (get-report-timezone-id-safely) + [expr db-type] (if report-timezone + [[:'now64 [:raw 9] (h2x/literal report-timezone)] (format "DateTime64(9, '%s')" report-timezone)] + [[:'now64 [:raw 9]] "DateTime64(9)"])] + (h2x/with-database-type-info expr db-type))) (defn- date-time-parse-fn [nano] @@ -124,9 +227,11 @@ (defmethod sql.qp/->honeysql [:clickhouse LocalDateTime] [_ ^java.time.LocalDateTime t] - (let [formatted (t/format "yyyy-MM-dd HH:mm:ss.SSS" t) - fn (date-time-parse-fn (.getNano t))] - [fn formatted])) + (let [formatted (t/format "yyyy-MM-dd HH:mm:ss.SSS" t) + report-timezone (h2x/literal (or (get-report-timezone-id-safely) "UTC"))] + (if (zero? (.getNano t)) + [:'parseDateTimeBestEffort formatted report-timezone] + [:'parseDateTime64BestEffort formatted 3 report-timezone]))) (defmethod sql.qp/->honeysql [:clickhouse ZonedDateTime] [_ ^java.time.ZonedDateTime t] @@ -315,22 +420,13 @@ (case unit ;; Week: Metabase tests expect a bit different result from what `age` provides (:week) - [:'intDiv [:'dateDiff (h2x/literal :day) [:'toStartOfDay x] [:'toStartOfDay y]] [:raw 7]] + [:'intDiv [:'dateDiff (h2x/literal :day) (date-trunc :'toStartOfDay x) (date-trunc :'toStartOfDay y)] [:raw 7]] ;; ------------------------- (:year :month :quarter :day) - [:'age (h2x/literal unit) [:'toStartOfDay x] [:'toStartOfDay y]] + [:'age (h2x/literal unit) (date-trunc :'toStartOfDay x) (date-trunc :'toStartOfDay y)] ;; ------------------------- (:hour :minute :second) - [:'age (h2x/literal unit) x y]))) - -;; FIXME: there are still many failing tests that prevent us from turning this feature on -;; (defmethod sql.qp/->honeysql [:clickhouse :convert-timezone] -;; [driver [_ arg target-timezone source-timezone]] -;; (let [expr (sql.qp/->honeysql driver (cond-> arg (string? arg) u.date/parse)) -;; with-tz-info? (h2x/is-of-type? expr #"(?:nullable\(|lowcardinality\()?(datetime64\(\d, {0,1}'.*|datetime\(.*)") -;; _ (sql.u/validate-convert-timezone-args with-tz-info? target-timezone source-timezone) -;; inner (if (not with-tz-info?) [:'toTimeZone expr source-timezone] expr)] -;; [:'toTimeZone inner target-timezone])) + [:'age (h2x/literal unit) (in-report-timezone x) (in-report-timezone y)]))) ;; We do not have Time data types, so we cheat a little bit (defmethod sql.qp/cast-temporal-string [:clickhouse :Coercion/ISO8601->Time] @@ -341,6 +437,10 @@ [_driver _special_type expr] expr) +;;; ------------------------------------------------------------------------------------ +;;; JDBC-related functions +;;; ------------------------------------------------------------------------------------ + (defmethod sql-jdbc.execute/read-column-thunk [:clickhouse Types/TINYINT] [_ ^ResultSet rs ^ResultSetMetaData _ ^Integer i] (fn [] @@ -368,21 +468,46 @@ (fn [] (with-null-check rs (.getInt rs i)))) +(defn- offset-date-time->maybe-offset-time + [^OffsetDateTime r] + (cond (nil? r) nil + (= (.toLocalDate r) (t/local-date 1970 1 1)) (.toOffsetTime r) + :else r)) + +(defn- local-date-time->maybe-local-time + [^LocalDateTime r] + (cond (nil? r) nil + (= (.toLocalDate r) (t/local-date 1970 1 1)) (.toLocalTime r) + :else r)) + +(defn- get-date-or-time-type + [tz-check-fn ^ResultSet rs ^Integer i] + (if (tz-check-fn) + (offset-date-time->maybe-offset-time (.getObject rs i OffsetDateTime)) + (local-date-time->maybe-local-time (.getObject rs i LocalDateTime)))) + +(defn- read-timestamp-column + [^ResultSet rs ^ResultSetMetaData rsmeta ^Integer i] + (let [db-type (remove-low-cardinality-and-nullable (u/lower-case-en (.getColumnTypeName rsmeta i)))] + (cond + ;; DateTime64 with tz info + (str/starts-with? db-type "datetime64") + (get-date-or-time-type #(> (count db-type) 13) rs i) + ;; DateTime with tz info + (str/starts-with? db-type "datetime") + (get-date-or-time-type #(> (count db-type) 8) rs i) + ;; _ + :else (.getObject rs i LocalDateTime)))) + (defmethod sql-jdbc.execute/read-column-thunk [:clickhouse Types/TIMESTAMP] - [_ ^ResultSet rs ^ResultSetMetaData _ ^Integer i] + [_ ^ResultSet rs ^ResultSetMetaData rsmeta ^Integer i] (fn [] - (let [^java.time.LocalDateTime r (.getObject rs i LocalDateTime)] - (cond (nil? r) nil - (= (.toLocalDate r) (t/local-date 1970 1 1)) (.toLocalTime r) - :else r)))) + (read-timestamp-column rs rsmeta i))) -;; FIXME: should be just (.getObject rs i OffsetDateTime) -;; still blocked by many failing tests (see `sql.qp/->honeysql [:clickhouse :convert-timezone]` as well) (defmethod sql-jdbc.execute/read-column-thunk [:clickhouse Types/TIMESTAMP_WITH_TIMEZONE] - [_ ^ResultSet rs ^ResultSetMetaData _ ^Integer i] + [_ ^ResultSet rs ^ResultSetMetaData rsmeta ^Integer i] (fn [] - (when-let [s (.getString rs i)] - (u.date/parse s)))) + (read-timestamp-column rs rsmeta i))) (defmethod sql-jdbc.execute/read-column-thunk [:clickhouse Types/TIME] [_ ^ResultSet rs ^ResultSetMetaData _ ^Integer i] diff --git a/test/metabase/driver/clickhouse_introspection_test.clj b/test/metabase/driver/clickhouse_introspection_test.clj index 3db586d..44b78f2 100644 --- a/test/metabase/driver/clickhouse_introspection_test.clj +++ b/test/metabase/driver/clickhouse_introspection_test.clj @@ -78,11 +78,11 @@ :clickhouse (testing "datetimes" (let [table-name "datetime_base_types"] - (is (= #{{:base-type :type/DateTime, + (is (= #{{:base-type :type/DateTimeWithTZ, :database-required false, :database-type "Nullable(DateTime('America/New_York'))", :name "c1"} - {:base-type :type/DateTime, + {:base-type :type/DateTimeWithTZ, :database-required true, :database-type "DateTime('America/New_York')", :name "c2"} @@ -94,11 +94,11 @@ :database-required true, :database-type "DateTime64(3)", :name "c4"} - {:base-type :type/DateTime, + {:base-type :type/DateTimeWithTZ, :database-required true, :database-type "DateTime64(9, 'America/New_York')", :name "c5"} - {:base-type :type/DateTime, + {:base-type :type/DateTimeWithTZ, :database-required false, :database-type "Nullable(DateTime64(6, 'America/New_York'))", :name "c6"} diff --git a/test/metabase/driver/clickhouse_temporal_bucketing_test.clj b/test/metabase/driver/clickhouse_temporal_bucketing_test.clj index b0625de..64dac84 100644 --- a/test/metabase/driver/clickhouse_temporal_bucketing_test.clj +++ b/test/metabase/driver/clickhouse_temporal_bucketing_test.clj @@ -12,7 +12,7 @@ ;; start_of_year == '2022-01-01 00:00:00' ;; mid_of_year == '2022-06-20 06:32:54' ;; end_of_year == '2022-12-31 23:59:59' -(deftest ^:parallel clickhouse-temporal-bucketing-server-tz +(deftest clickhouse-temporal-bucketing-server-tz (mt/test-driver :clickhouse (defn- start-of-year [unit] @@ -102,7 +102,7 @@ (is (= [[4]] (end-of-year :quarter-of-year))))))) -(deftest ^:parallel clickhouse-temporal-bucketing-column-tz +(deftest clickhouse-temporal-bucketing-column-tz (mt/test-driver :clickhouse (defn- start-of-year [unit] @@ -131,15 +131,13 @@ {:breakout [[:field %end_of_year {:temporal-unit unit}]]})))))) (testing "truncate to" (testing "minute" - ;; it's actually not in UTC as the suffix suggests - ;; however, it is still rendered correctly on the UI - (is (= [["2022-06-20T06:32:00Z"]] + (is (= [["2022-06-20T13:32:00Z"]] (mid-year :minute)))) (testing "hour" - (is (= [["2022-06-20T06:00:00Z"]] + (is (= [["2022-06-20T13:00:00Z"]] (mid-year :hour)))) (testing "day" - (is (= [["2022-06-20T00:00:00Z"]] + (is (= [["2022-06-20T07:00:00Z"]] (mid-year :day)))) (testing "month" (is (= [["2022-06-01T00:00:00Z"]] diff --git a/test/metabase/driver/clickhouse_test.clj b/test/metabase/driver/clickhouse_test.clj index 2425429..af1fbde 100644 --- a/test/metabase/driver/clickhouse_test.clj +++ b/test/metabase/driver/clickhouse_test.clj @@ -1,12 +1,10 @@ (ns metabase.driver.clickhouse-test "Tests for specific behavior of the ClickHouse driver." #_{:clj-kondo/ignore [:unsorted-required-namespaces]} - (:require [cljc.java-time.format.date-time-formatter :as date-time-formatter] - [cljc.java-time.offset-date-time :as offset-date-time] - [cljc.java-time.temporal.chrono-unit :as chrono-unit] - [clojure.test :refer :all] + (:require [clojure.test :refer :all] [metabase.driver :as driver] [metabase.driver.clickhouse :as clickhouse] + [metabase.driver.clickhouse-qp :as clickhouse-qp] [metabase.driver.clickhouse-data-types-test] [metabase.driver.clickhouse-impersonation-test] [metabase.driver.clickhouse-introspection-test] @@ -14,9 +12,7 @@ [metabase.driver.clickhouse-temporal-bucketing-test] [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn] [metabase.models.database :refer [Database]] - [metabase.query-processor :as qp] [metabase.query-processor.compile :as qp.compile] - [metabase.query-processor.test-util :as qp.test] [metabase.test :as mt] [metabase.test.data :as data] [metabase.test.data.interface :as tx] @@ -48,21 +44,6 @@ spec (sql-jdbc.conn/connection-details->spec :clickhouse details)] (driver/db-default-timezone :clickhouse spec)))))) -(deftest ^:parallel clickhouse-now-converted-to-timezone - (mt/test-driver - :clickhouse - (let [[[utc-now shanghai-now]] - (qp.test/rows - (qp/process-query - (mt/native-query - {:query "SELECT now(), now('Asia/Shanghai')"})))] - (testing "there is always eight hour difference in time between UTC and Asia/Beijing" - (is (= 8 - (chrono-unit/between - chrono-unit/hours - (offset-date-time/parse utc-now date-time-formatter/iso-offset-date-time) - (offset-date-time/parse shanghai-now date-time-formatter/iso-offset-date-time)))))))) - (deftest ^:parallel clickhouse-connection-string (mt/with-dynamic-redefs [;; This function's implementation requires the connection details to actually connect to the ;; database, which is orthogonal to the purpose of this test. @@ -189,3 +170,14 @@ (let [details (merge {:user username :password password} (tx/dbdef->connection-details :clickhouse :db {:database-name database}))] (is (true? (driver/can-connect? :clickhouse details))))))))) + +(deftest clickhouse-qp-extract-datetime-timezone + (mt/test-driver + :clickhouse + (is (= "utc" (#'clickhouse-qp/extract-datetime-timezone "datetime('utc')"))) + (is (= "utc" (#'clickhouse-qp/extract-datetime-timezone "datetime64(3, 'utc')"))) + (is (= "europe/amsterdam" (#'clickhouse-qp/extract-datetime-timezone "datetime('europe/amsterdam')"))) + (is (= "europe/amsterdam" (#'clickhouse-qp/extract-datetime-timezone "datetime64(9, 'europe/amsterdam')"))) + (is (= nil (#'clickhouse-qp/extract-datetime-timezone "datetime"))) + (is (= nil (#'clickhouse-qp/extract-datetime-timezone "datetime64"))) + (is (= nil (#'clickhouse-qp/extract-datetime-timezone "datetime64(3)"))))) diff --git a/test/metabase/test/data/clickhouse.clj b/test/metabase/test/data/clickhouse.clj index e8ea91b..2a0f3a1 100644 --- a/test/metabase/test/data/clickhouse.clj +++ b/test/metabase/test/data/clickhouse.clj @@ -37,17 +37,21 @@ :http_connection_provider "HTTP_URL_CONNECTION" :custom_http_params "allow_experimental_analyzer=0"}) -(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Boolean] [_ _] "Boolean") -(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/BigInteger] [_ _] "Int64") -(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Char] [_ _] "String") -(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Date] [_ _] "Date") -(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/DateTime] [_ _] "DateTime64") -(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Float] [_ _] "Float64") -(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Integer] [_ _] "Int32") -(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/IPAddress] [_ _] "IPv4") -(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Text] [_ _] "String") -(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/UUID] [_ _] "UUID") -(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Time] [_ _] "DateTime64") +;; (def ^:private time-type-comment "COMMENT 'time'") +;; (def ^:private time-type (format "DateTime64(3) %s" time-type-comment)) + +(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Boolean] [_ _] "Boolean") +(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/BigInteger] [_ _] "Int64") +(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Char] [_ _] "String") +(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Date] [_ _] "Date") +(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/DateTime] [_ _] "DateTime64") +(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/DateTimeWithTZ] [_ _] "DateTime64(3, 'UTC')") +(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Float] [_ _] "Float64") +(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Integer] [_ _] "Int32") +(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/IPAddress] [_ _] "IPv4") +(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Text] [_ _] "String") +(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/UUID] [_ _] "UUID") +(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Time] [_ _] "Time") (defmethod tx/sorts-nil-first? :clickhouse [_ _] false) @@ -91,8 +95,14 @@ (:native base-type) (sql.tx/field-base-type->sql-type :clickhouse base-type)) col-name (quote-name field-name) - fmt (if (or pk? (disallowed-as-nullable? ch-type)) "%s %s" "%s Nullable(%s)")] - (format fmt col-name ch-type))) + ch-col (cond + (or pk? (disallowed-as-nullable? ch-type)) + (format "%s %s" col-name ch-type) + (= ch-type "Time") + (format "%s Nullable(DateTime64) COMMENT 'time'" col-name) + ; _ + :else (format "%s Nullable(%s)" col-name ch-type))] + ch-col)) (defn- ->comma-separated-str [coll]