Prepare 1.2.2 (#194)

* Remove unnecessary CAST as timestamp calls * Disable impersonation due to cluster setup issues * Remove forward slash from serialized IPv4/IPv6 * Move introspection code out of the main file * Bump versions * Update tests * Add DateTimeWithTZ experimental stuff (commented out for now).
ClickHouse · Sep 29, 2023 · b199284 · b199284
1 parent 0b724a1
commit b199284
Show file tree

Hide file tree

Showing 14 changed files with 510 additions and 334 deletions.
diff --git a/.docker/clickhouse/single_node_tls/Dockerfile b/.docker/clickhouse/single_node_tls/Dockerfile
@@ -1,4 +1,4 @@
-FROM clickhouse/clickhouse-server:23.5-alpine
+FROM clickhouse/clickhouse-server:23.8-alpine
 COPY .docker/clickhouse/single_node_tls/certificates /etc/clickhouse-server/certs
 RUN chown clickhouse:clickhouse -R /etc/clickhouse-server/certs \
     && chmod 600 /etc/clickhouse-server/certs/* \

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,9 @@
+# 1.2.2
+
+### Bug fixes
+* Removed forward slash from serialized IPv4/IPv6 columns. NB: IPv4/IPv6 columns are temporarily resolved as `type/TextLike` instead of `type/IPAddress` base type due to an unexpected result in Metabase 0.47 type check.
+* Removed superfluous CAST calls from generated queries that use Date* columns and/or intervals
+
 # 1.2.1
 ### New features
 * Use HoneySQL2 in the driver

diff --git a/README.md b/README.md
@@ -32,11 +32,11 @@
 6. Make sure you are the in the directory where your `metabase.jar` lives.
 7. Run `MB_PLUGINS_DIR=./plugins; java -jar metabase.jar`.
 
-For example [(using Metabase v0.45.3 and ClickHouse driver 1.1.2)](#choosing-the-right-version):
+For example [(using Metabase v0.47.2 and ClickHouse driver 1.2.2)](#choosing-the-right-version):
 
 ```bash
 export METABASE_VERSION=v0.47.2
-export METABASE_CLICKHOUSE_DRIVER_VERSION=1.2.1
+export METABASE_CLICKHOUSE_DRIVER_VERSION=1.2.2
 
 mkdir -p mb/plugins && cd mb
 curl -o metabase.jar https://downloads.metabase.com/$METABASE_VERSION/metabase.jar
@@ -50,7 +50,7 @@ Alternatively, if you don't want to run Metabase Jar, you can use a Docker image
 
 ```bash
 export METABASE_DOCKER_VERSION=v0.47.2
-export METABASE_CLICKHOUSE_DRIVER_VERSION=1.2.1
+export METABASE_CLICKHOUSE_DRIVER_VERSION=1.2.2
 
 mkdir -p mb/plugins && cd mb
 curl -L -o plugins/ch.jar https://github.com/ClickHouse/metabase-clickhouse-driver/releases/download/$METABASE_CLICKHOUSE_DRIVER_VERSION/clickhouse.metabase-driver.jar
@@ -74,17 +74,17 @@ docker run -d -p 3000:3000 \
 | 0.44.x           | 0.9.1          |
 | 0.45.x           | 1.1.0          |
 | 0.46.x           | 1.1.7          |
-| 0.47.x           | 1.2.1          |
+| 0.47.x           | 1.2.2          |
 
 ## Creating a Metabase Docker image with ClickHouse driver
 
 You can use a convenience script `build_docker_image.sh`, which takes three arguments: Metabase version, ClickHouse driver version, and the desired final Docker image tag.
 
 ```bash
-./build_docker_image.sh v0.44.6 0.8.3 my-metabase-with-clickhouse:v0.0.1
+./build_docker_image.sh v0.47.2 1.2.2 my-metabase-with-clickhouse:v0.0.1
 ```
 
-where `v0.44.6` is Metabase version, `0.8.3` is ClickHouse driver version, and `my-metabase-with-clickhouse:v0.0.1` being the tag.
+where `v0.47.2` is Metabase version, `1.2.2` is ClickHouse driver version, and `my-metabase-with-clickhouse:v0.0.1` being the tag.
 
 Then you should be able to run it:
 
@@ -98,7 +98,7 @@ or use it with Docker compose, for example:
 version: '3.8'
 services:
   clickhouse:
-    image: 'clickhouse/clickhouse-server:22.10.2-alpine'
+    image: 'clickhouse/clickhouse-server:23.8-alpine'
     container_name: 'metabase-clickhouse-server'
     ports:
       - '8123:8123'

diff --git a/build_docker_image.sh b/build_docker_image.sh
@@ -12,7 +12,7 @@ if [ $# -lt 3 ]; then
     exit 1
 fi
 
-export DOWNLOAD_URL="https://github.com/enqueue/metabase-clickhouse-driver/releases/download/$2/clickhouse.metabase-driver.jar"
+export DOWNLOAD_URL="https://github.com/ClickHouse/metabase-clickhouse-driver/releases/download/$2/clickhouse.metabase-driver.jar"
 echo "Downloading the driver from $DOWNLOAD_URL"
 
 cd .build

diff --git a/docker-compose.yml b/docker-compose.yml
@@ -1,7 +1,7 @@
 version: '3.8'
 services:
   clickhouse:
-    image: 'clickhouse/clickhouse-server:${CLICKHOUSE_VERSION-23.5-alpine}'
+    image: 'clickhouse/clickhouse-server:23.8-alpine'
     container_name: 'metabase-driver-clickhouse-server'
     ports:
       - '8123:8123'
@@ -32,7 +32,7 @@ services:
     hostname: server.clickhouseconnect.test
 
   metabase:
-    image: metabase/metabase:v0.47.0-RC2
+    image: metabase/metabase:v0.47.2
     container_name: metabase-with-clickhouse-driver
     environment:
       'MB_HTTP_TIMEOUT': '5000'

diff --git a/resources/metabase-plugin.yaml b/resources/metabase-plugin.yaml
@@ -1,6 +1,6 @@
 info:
   name: Metabase ClickHouse Driver
-  version: 1.2.1
+  version: 1.2.2
   description: Allows Metabase to connect to ClickHouse databases.
 contact-info:
   name: ClickHouse

diff --git a/src/metabase/driver/clickhouse.clj b/src/metabase/driver/clickhouse.clj
@@ -1,74 +1,37 @@
 (ns metabase.driver.clickhouse
   "Driver for ClickHouse databases"
-  #_{:clj-kondo/ignore [:unsorted-required-namespaces]}
   (:require [clojure.java.jdbc :as jdbc]
             [clojure.string :as str]
-            [metabase [config :as config] [driver :as driver] [util :as u]]
-            [metabase.driver.clickhouse-qp]
+            [metabase.driver :as driver]
+            [metabase.driver.clickhouse-introspection]
             [metabase.driver.clickhouse-nippy]
+            [metabase.driver.clickhouse-qp]
             [metabase.driver.ddl.interface :as ddl.i]
             [metabase.driver.sql :as driver.sql]
             [metabase.driver.sql-jdbc [common :as sql-jdbc.common]
              [connection :as sql-jdbc.conn]
-             [sync :as sql-jdbc.sync]])
-  (:import (java.sql DatabaseMetaData)))
+             [sync :as sql-jdbc.sync]]
+            [metabase [config :as config]]))
 
 (set! *warn-on-reflection* true)
 
 (driver/register! :clickhouse :parent :sql-jdbc)
 
-(def ^:private database-type->base-type
-  (sql-jdbc.sync/pattern-based-database-type->base-type
-   [[#"Array" :type/Array]
-    [#"Bool" :type/Boolean]
-    ;; TODO: test it with :type/DateTimeWithTZ
-    [#"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]]))
-
-;; Enum8(UInt8) -> Enum8, DateTime64(Europe/Amsterdam) -> DateTime64,
-;; Nullable(DateTime) -> DateTime, SimpleAggregateFunction(sum, Int64) -> Int64, etc
-(defn- ^:private normalize-database-type
-  [database-type]
-  (let [db-type    (subs (str database-type) 1) ;; keyword->str; `name` call does not work well
-        normalized (second (re-find #"(?:Nullable\(|LowCardinality\()?(\w+)?\({0,1}.*" db-type))]
-    ;; slightly different normalization for SimpleAggregateFunction - we need to take the second arg
-    (or (keyword (if (= normalized "SimpleAggregateFunction")
-                   (second (re-find #"SimpleAggregateFunction\(\w+?, {0,1}(.+)?\)" db-type))
-                   normalized))
-        database-type))) ;; basically, fall back to :type/* later
+(defmethod driver/display-name :clickhouse [_] "ClickHouse")
+(def ^:private product-name "metabase/1.2.2")
 
-(defmethod sql-jdbc.sync/database-type->base-type :clickhouse
-  [_ database-type]
-  (database-type->base-type (normalize-database-type database-type)))
+(doseq [[feature supported?] {:standard-deviation-aggregations true
+                              :foreign-keys                    (not config/is-test?)
+                              :set-timezone                    false
+                              :convert-timezone                false
+                              :test/jvm-timezone-setting       false
+                              :connection-impersonation        false
+                              :schemas                         true}]
 
-(def ^:private excluded-schemas #{"system" "information_schema" "INFORMATION_SCHEMA"})
-(defmethod sql-jdbc.sync/excluded-schemas :clickhouse [_] excluded-schemas)
+  (defmethod driver/database-supports? [:clickhouse feature] [_driver _feature _db] supported?))
 
 (def ^:private default-connection-details
-  {:user "default", :password "", :dbname "default", :host "localhost", :port "8123"})
-(def ^:private product-name "metabase/1.2.1")
+  {:user "default" :password "" :dbname "default" :host "localhost" :port "8123"})
 
 (defmethod sql-jdbc.conn/connection-details->spec :clickhouse
   [_ details]
@@ -89,104 +52,6 @@
       :product_name product-name}
      (sql-jdbc.common/handle-additional-options details :separator-style :url))))
 
-(def ^:private allowed-table-types
-  (into-array String
-              ["TABLE" "VIEW" "FOREIGN TABLE" "REMOTE TABLE" "DICTIONARY"
-               "MATERIALIZED VIEW" "MEMORY TABLE" "LOG TABLE"]))
-
-(defn- tables-set
-  [tables]
-  (set
-   (for [table tables]
-     (let [remarks (:remarks table)]
-       {:name (:table_name table)
-        :schema (:table_schem table)
-        :description (when-not (str/blank? remarks) remarks)}))))
-
-(defn- get-tables-from-metadata
-  [^DatabaseMetaData metadata schema-pattern]
-  (.getTables metadata
-              nil            ; catalog - unused in the source code there
-              schema-pattern
-              "%"            ; tablePattern "%" = match all tables
-              allowed-table-types))
-
-(defn ^:private not-inner-mv-table?
-  [table]
-  (not (str/starts-with? (:table_name table) ".inner")))
-
-(defn- ->spec
-  [db]
-  (if (u/id db)
-    (sql-jdbc.conn/db->pooled-connection-spec db) db))
-
-(defn- get-all-tables
-  [db]
-  (jdbc/with-db-metadata [metadata (->spec db)]
-    (->> (get-tables-from-metadata metadata "%")
-         (jdbc/metadata-result)
-         (vec)
-         (filter #(and
-                   (not (contains? excluded-schemas (:table_schem %)))
-                   (not-inner-mv-table? %)))
-         (tables-set))))
-
-;; Strangely enough, the tests only work with :db keyword,
-;; but the actual sync from the UI uses :dbname
-(defn- get-db-name
-  [db]
-  (or (get-in db [:details :dbname])
-      (get-in db [:details :db])))
-
-(defn- get-tables-in-dbs [db-or-dbs]
-  (->> (for [db (as-> (or (get-db-name db-or-dbs) "default") dbs
-                  (str/split dbs #" ")
-                  (remove empty? dbs)
-                  (map (comp #(ddl.i/format-name :clickhouse %) str/trim) dbs))]
-         (jdbc/with-db-metadata [metadata (->spec db-or-dbs)]
-           (jdbc/metadata-result
-            (get-tables-from-metadata metadata db))))
-       (apply concat)
-       (filter not-inner-mv-table?)
-       (tables-set)))
-
-(defmethod driver/describe-database :clickhouse
-  [_ {{:keys [scan-all-databases]}
-      :details :as db}]
-  {:tables
-   (if
-    (boolean scan-all-databases)
-     (get-all-tables db)
-     (get-tables-in-dbs db))})
-
-(defn- ^:private is-db-required?
-  [field]
-  (not (str/starts-with? (get-in field [:database-type]) "Nullable")))
-
-(defmethod driver/describe-table :clickhouse
-  [_ database table]
-  (let [table-metadata (sql-jdbc.sync/describe-table :clickhouse database table)
-        filtered-fields (for [field (:fields table-metadata)
-                              :let [updated-field (update-in field [:database-required]
-                                                             (fn [_] (is-db-required? field)))]
-                              ;; Skip all AggregateFunction (but keeping SimpleAggregateFunction) columns
-                              ;; JDBC does not support that and it crashes the data browser
-                              :when (not (re-matches #"^AggregateFunction\(.+$"
-                                                     (get field :database-type)))]
-                          updated-field)]
-    (merge table-metadata {:fields (set filtered-fields)})))
-
-(defmethod driver/display-name :clickhouse [_] "ClickHouse")
-
-(doseq [[feature supported?] {:standard-deviation-aggregations true
-                              :set-timezone                    false
-                              :foreign-keys                    (not config/is-test?)
-                              :test/jvm-timezone-setting       false
-                              :connection-impersonation        true
-                              :schemas                         true}]
-
-  (defmethod driver/database-supports? [:clickhouse feature] [_driver _feature _db] supported?))
-
 (defmethod sql-jdbc.sync/db-default-timezone :clickhouse
   [_ spec]
   (let [sql (str "SELECT timezone() AS tz")