Integration 0412 (#76)

ClickHouse · Dec 4, 2021 · 6ec9859 · 6ec9859
1 parent 3673d60
commit 6ec9859
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 128 deletions.
diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml
@@ -1,6 +1,10 @@
 name: Check
 
-on: [pull_request, push]
+on:
+  push:
+    branches:
+      - master
+  pull_request:
 
 jobs:
   check:
@@ -16,45 +20,52 @@ jobs:
           TZ: "/usr/share/zoneinfo/UTC"
 
     steps:
-      - name: Prepare JDK 11
-        uses: actions/setup-java@v1
-        with:
-          java-version: 11
       - name: Checkout Metabase Repo
         uses: actions/checkout@v2
         with:
           repository: enqueue/metabase
-          ref: clickhouse_driver_testing
+          ref: metabase_0.41.2
       - name: Checkout Driver Repo
         uses: actions/checkout@v2
         with:
           path: modules/drivers/clickhouse
-      - name: Cache Dependencies
-        uses: actions/cache@v2
-        env:
-          cache-name: cache-metabase-deps
+      - name: Prepare JDK 11
+        uses: actions/setup-java@v2
         with:
-          path: ~/.m2
-          key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/project.clj', '**/package.json', '**/pom.xml') }}
+          distribution: 'temurin'
+          java-version: '11'
       - name: Install Clojure CLI
         run: |
-          curl -O https://download.clojure.org/install/linux-install-1.10.1.708.sh &&
-          sudo bash ./linux-install-1.10.1.708.sh
+          curl -O https://download.clojure.org/install/linux-install-1.10.3.1029.sh &&
+          sudo bash ./linux-install-1.10.3.1029.sh
+      - name: Setup Node
+        uses: actions/setup-node@v2
+        with:
+          node-version: '14'
+          cache: 'yarn'
+      - name: Get M2 cache
+        uses: actions/cache@v2
+        with:
+          path: |
+            ~/.m2
+            ~/.gitlibs
+          key: ${{ runner.os }}-clickhouse-${{ hashFiles('**/deps.edn') }}
       - name: Build ClickHouse driver
         run: |
           bin/build-driver.sh clickhouse
       - name: Archive driver JAR
         uses: actions/upload-artifact@v2
         with:
-          name: Driver JAR
-          path: modules/drivers/clickhouse/target/uberjar/clickhouse.metabase-driver.jar
+          name: clickhouse.metabase-driver.jar
+          path: resources/modules/clickhouse.metabase-driver.jar
+      - name: Prepare stuff for pulses
+        run: yarn build-static-viz
       - name: Run tests
-        env:
-          DRIVERS: h2,clickhouse
         run:
-          lein with-profile +ci,+junit test
+          DRIVERS=clickhouse clojure -X:dev:drivers:drivers-dev:test
       - name: Report test results
-        uses: mikepenz/action-junit-report@v2
+        uses: mikepenz/action-junit-report@v2.8.1
         if: always()
         with:
-          report_paths: '**/target/junit/TEST-*.xml'
+          report_paths: '**/target/junit/*.xml'
+          github_token: ${{ secrets.GITHUB_TOKEN }}
diff --git a/deps.edn b/deps.edn
@@ -0,0 +1,5 @@
+{:paths
+ ["src" "resources"]
+
+ :deps
+ {ru.yandex.clickhouse/clickhouse-jdbc {:mvn/version "0.3.1"}}}
diff --git a/project.clj b/project.clj
diff --git a/src/metabase/driver/clickhouse.clj b/src/metabase/driver/clickhouse.clj
@@ -22,7 +22,7 @@
             [metabase.util.honeysql-extensions :as hx]
             [schema.core :as s])
   (:import [ru.yandex.clickhouse.util ClickHouseArrayUtil]
-           [java.sql DatabaseMetaData PreparedStatement ResultSet Time Types]
+           [java.sql DatabaseMetaData PreparedStatement ResultSet ResultSetMetaData Time Types]
            [java.time Instant LocalDate LocalDateTime LocalTime OffsetDateTime OffsetTime ZonedDateTime]
            [java.time.temporal Temporal]
            [java.util TimeZone]))
@@ -37,8 +37,8 @@
     [#"DateTime64"  :type/DateTime]
     [#"Date"        :type/Date]
     [#"Decimal"     :type/Decimal]
-    [#"Enum8"       :type/Enum]
-    [#"Enum16"      :type/Enum]
+    [#"Enum8"       :type/Text]
+    [#"Enum16"      :type/Text]
     [#"FixedString" :type/TextLike]
     [#"Float32"     :type/Float]
     [#"Float64"     :type/Float]
@@ -61,7 +61,9 @@
    (str/replace (name database-type) #"(?:Nullable|LowCardinality)\((\S+)\)" "$1")))
 
 (defmethod sql-jdbc.sync/excluded-schemas :clickhouse [_]
-  #{"system"})
+  #{"system"
+    "information_schema"
+    "INFORMATION_SCHEMA"})
 
 (defmethod sql-jdbc.conn/connection-details->spec :clickhouse
   [_ {:keys [user password dbname host port ssl]
@@ -74,10 +76,7 @@
        :user                           user
        :ssl                            (boolean ssl)
        :use_server_time_zone_for_dates true}
-      (sql-jdbc.common/handle-additional-options details, :seperator-style :url)))
-
-(defn- modulo [a b]
-  (hsql/call :modulo a b))
+      (sql-jdbc.common/handle-additional-options details, :separator-style :url)))
 
 (defn- to-relative-day-num [expr]
   (hsql/call :toRelativeDayNum (hsql/call :toDateTime expr)))
@@ -130,9 +129,9 @@
 (defn- to-day [expr]
   (hsql/call :toDate expr))
 
-(defn- to-day-of-week [expr]
-  ;; ClickHouse weeks usually start on Monday
-  (hx/+ (modulo (hsql/call :toDayOfWeek (hsql/call :toDateTime expr)) 7) 1))
+(defmethod sql.qp/date [:clickhouse :day-of-week]
+  [_ _ expr]
+  (sql.qp/adjust-day-of-week :clickhouse (hsql/call :dayOfWeek expr)))
 
 (defn- to-day-of-month [expr]
   (hsql/call :toDayOfMonth (hsql/call :toDateTime expr)))
@@ -148,7 +147,6 @@
 (defmethod sql.qp/date [:clickhouse :minute-of-hour]  [_ _ expr] (to-minute expr))
 (defmethod sql.qp/date [:clickhouse :hour]            [_ _ expr] (to-start-of-hour expr))
 (defmethod sql.qp/date [:clickhouse :hour-of-day]     [_ _ expr] (to-hour expr))
-(defmethod sql.qp/date [:clickhouse :day-of-week]     [_ _ expr] (to-day-of-week expr))
 (defmethod sql.qp/date [:clickhouse :day-of-month]    [_ _ expr] (to-day-of-month expr))
 (defmethod sql.qp/date [:clickhouse :day-of-year]     [_ _ expr] (to-day-of-year expr))
 (defmethod sql.qp/date [:clickhouse :week-of-year]    [_ _ expr] (to-week-of-year expr))
@@ -176,9 +174,10 @@
   [_ t]
   (format "'%s'" (t/format "HH:mm:ss.SSSZZZZZ" t)))
 
+
 (defmethod unprepare/unprepare-value [:clickhouse LocalDateTime]
   [_ t]
-  (format "parseDateTimeBestEffort('%s')" (t/format "yyyy-MM-dd HH:mm:ss.SSS" t)))
+  (format "'%s'" (t/format "yyyy-MM-dd HH:mm:ss.SSS" t)))
 
 (defmethod unprepare/unprepare-value [:clickhouse OffsetDateTime]
   [_ t]
@@ -245,12 +244,6 @@
     (hsql/call :count (sql.qp/->honeysql driver field))
     :%count))
 
-;; better performance than count(distinct ...) which gets
-;; translated into uniqExact
-(defmethod sql.qp/->honeysql [:clickhouse :distinct]
-  [driver [_ field]]
-  (hsql/call :uniq (sql.qp/->honeysql driver field)))
-
 (defmethod hformat/fn-handler "quantile"
   [_ field p]
   (str "quantile("
@@ -275,6 +268,7 @@
   [driver [_ arg pattern]]
   (hsql/call :extract_ch (sql.qp/->honeysql driver arg) pattern))
 
+;; we still need this for decimal versus float
 (defmethod sql.qp/->honeysql [:clickhouse :/]
   [driver args]
   (let [args (for [arg args]
@@ -351,22 +345,23 @@
   (ch-like-clause driver (sql.qp/->honeysql driver field) (update-string-value value #(str \% %)) options))
 
 ;; We do not have Time data types, so we cheat a little bit
-(defmethod sql.qp/cast-temporal-string [:clickhouse :type/ISO8601TimeString]
+(defmethod sql.qp/cast-temporal-string [:clickhouse :Coercion/ISO8601->Time]
   [_driver _special_type expr]
-  (hsql/call :parseDateTimeBestEffort (hsql/call :concat "1970-01-01T" expr)))
+  (hx/->timestamp (hsql/call :parseDateTimeBestEffort (hsql/call :concat "1970-01-01T", expr))))
 
-(defmethod sql.qp/cast-temporal-string [:clickhouse :type/ISO8601DateString]
-  [_driver _semantic_type expr]
-  (hx/->date expr))
+(defmethod sql.qp/cast-temporal-byte [:clickhouse :Coercion/ISO8601->Time]
+  [_driver _special_type expr]
+  (hx/->timestamp expr))
 
-(defmethod sql-jdbc.execute/read-column [:clickhouse Types/TIMESTAMP] [_ _ rs _ i]
-  (let [r (.getObject rs i OffsetDateTime)]
-    (cond
-      (nil? r) nil
-      (= (.toLocalDate r) (t/local-date 1970 1 1)) (.toOffsetTime r)
-      :else r)))
+(defmethod sql-jdbc.execute/read-column-thunk [:clickhouse Types/TIMESTAMP] [_ ^ResultSet rs ^ResultSetMetaData rsmeta ^Integer i]
+  (fn []
+    (let [r (.getObject rs i LocalDateTime)]
+      (cond
+        (nil? r) nil
+        (= (.toLocalDate r) (t/local-date 1970 1 1)) (.toLocalTime r)
+        :else r))))
 
-(defmethod sql-jdbc.execute/read-column [:clickhouse Types/TIME] [_ _ rs _ i]
+(defmethod sql-jdbc.execute/read-column-thunk [:clickhouse Types/TIME] [_ ^ResultSet rs ^ResultSetMetaData rsmeta ^Integer i]
   (.getObject rs i OffsetTime))
 
 (defmethod sql-jdbc.execute/read-column [:clickhouse Types/ARRAY] [driver calendar resultset meta i]
@@ -421,10 +416,9 @@
 
 (defmethod driver/supports? [:clickhouse :standard-deviation-aggregations] [_ _] true)
 
-(defmethod driver/db-default-timezone :clickhouse
-  [_ db]
-  (let [spec                             (sql-jdbc.conn/db->pooled-connection-spec db)
-        sql                              (str "SELECT timezone() AS tz")
+(defmethod sql-jdbc.sync/db-default-timezone :clickhouse
+  [_ spec]
+  (let [sql            (str "SELECT timezone() AS tz")
         [{:keys [tz]}] (jdbc/query spec sql)]
     tz))
 

diff --git a/test/metabase/driver/clickhouse_test.clj b/test/metabase/driver/clickhouse_test.clj
@@ -17,10 +17,6 @@
             [metabase.test.util :as tu]
             [toucan.util.test :as tt]))
 
-(deftest clickhouse-timezone-is-utc
-  (mt/test-driver :clickhouse
-                  (is (= "UTC"
-                         (tu/db-timezone-id)))))
 
 (deftest clickhouse-decimal-division-simple
   (mt/test-driver :clickhouse
@@ -178,11 +174,11 @@
                   (is (= {:name   "enums_test"
                           :fields #{{:name              "enum1"
                                      :database-type     "Enum8"
-                                     :base-type         :type/Enum
+                                     :base-type         :type/Text
                                      :database-position 0}
                                     {:name              "enum2"
                                      :database-type     "Enum16"
-                                     :base-type         :type/Enum
+                                     :base-type         :type/Text
                                      :database-position 1}}}
                          (do-with-enums-db
                           (fn [db]

diff --git a/test/metabase/test/data/clickhouse.clj b/test/metabase/test/data/clickhouse.clj
@@ -17,14 +17,14 @@
 (defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Boolean]    [_ _] "UInt8")
 (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]   [_ _] "DateTime")
+(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]    [_ _] "Nullable(Int32)")
 (defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Text]       [_ _] "Nullable(String)")
 (defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/UUID]       [_ _] "UUID")
-(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Time]       [_ _] "DateTime")
+(defmethod sql.tx/field-base-type->sql-type [:clickhouse :type/Time]       [_ _] "DateTime64")
 
-(defmethod tx/sorts-nil-first? :clickhouse [_] false)
+(defmethod tx/sorts-nil-first? :clickhouse [_ _] false)
 
 (defmethod tx/dbdef->connection-details :clickhouse [_ context {:keys [database-name]}]
     (merge