Fix boolean array type introspection (#259)

* Fix tests namespaces, prevent CH tests to run multiple times

* Simplify the boolean array type introspection

* Bump versions, update CHANGELOG

CHANGELOG.md

@@ -1,3 +1,9 @@
+# 1.50.2
+
+### Bug fixes
+
+* Fixed Array inner type introspection, which could cause reduced performance when querying tables containing arrays. ([#257](https://github.com/ClickHouse/metabase-clickhouse-driver/issues/257))
+
 # 1.50.1
 
 ### New features

resources/metabase-plugin.yaml

@@ -1,6 +1,6 @@
 info:
   name: Metabase ClickHouse Driver
-  version: 1.50.1
+  version: 1.50.2
   description: Allows Metabase to connect to ClickHouse databases.
 contact-info:
   name: ClickHouse

src/metabase/driver/clickhouse.clj

@@ -29,7 +29,7 @@
 (driver/register! :clickhouse :parent #{:sql-jdbc})
 
 (defmethod driver/display-name :clickhouse [_] "ClickHouse")
-(def ^:private product-name "metabase/1.50.1")
+(def ^:private product-name "metabase/1.50.2")
 
 (defmethod driver/prettify-native-form :clickhouse
   [_ native-form]

src/metabase/driver/clickhouse_qp.clj

@@ -521,25 +521,17 @@
       (.getLong rs i)
       (.getBigDecimal rs i))))
 
-(defn- get-array-base-type
-  [^ResultSetMetaData rsmeta ^Integer i]
-  (try
-    (-> (.columns rsmeta)
-        (.get i)
-        (.arrayBaseColumn)
-        (.originalTypeName))
-    (catch Exception e
-      (log/error e "Failed to get the inner type of the array column"))))
-
 (defmethod sql-jdbc.execute/read-column-thunk [:clickhouse Types/ARRAY]
   [_ ^ResultSet rs ^ResultSetMetaData rsmeta ^Integer i]
   (fn []
     (when-let [arr (.getArray rs i)]
-      (let [array-base-type (get-array-base-type rsmeta i)
-            inner           (.getArray arr)]
+      (let [col-type-name (.getColumnTypeName rsmeta i)
+            inner         (.getArray arr)]
         (cond
-          (= array-base-type "Bool")
+          (= "Bool" col-type-name)
           (str "[" (str/join ", " (map #(if (= 1 %) "true" "false") inner)) "]")
+          (= "Nullable(Bool)" col-type-name)
+          (str "[" (str/join ", " (map #(cond (= 1 %) "true" (= 0 %) "false" :else "null") inner)) "]")
           ;; All other primitives
           (.isPrimitive (.getComponentType (.getClass inner)))
           (Arrays/toString inner)

test/metabase/driver/clickhouse_data_types_test.clj

@@ -1,4 +1,4 @@
-(ns metabase.driver.clickhouse-test
+(ns metabase.driver.clickhouse-data-types-test
   #_{:clj-kondo/ignore [:unsorted-required-namespaces]}
   (:require [cljc.java-time.local-date :as local-date]
             [cljc.java-time.offset-date-time :as offset-date-time]
@@ -9,6 +9,8 @@
             [metabase.test.data.interface :as tx]
             [metabase.test.data.clickhouse :as ctd]))
 
+(use-fixtures :once ctd/create-test-db!)
+
 (deftest ^:parallel clickhouse-decimals
   (mt/test-driver
    :clickhouse
@@ -129,6 +131,21 @@
          result (ctd/rows-without-index query-result)]
      (is (= [["[true, false, true]"], ["[]"]] result)))))
 
+(deftest ^:parallel clickhouse-array-of-nullable-booleans
+  (mt/test-driver
+   :clickhouse
+   (let [row1 (into-array (list true false nil))
+         row2 (into-array nil)
+         query-result (data/dataset
+                       (tx/dataset-definition "metabase_tests_array_of_nullable_booleans"
+                                              ["test-data-array-of-booleans"
+                                               [{:field-name "my_array_of_nullable_booleans"
+                                                 :base-type {:native "Array(Nullable(Boolean))"}}]
+                                               [[row1] [row2]]])
+                       (data/run-mbql-query test-data-array-of-booleans {}))
+         result (ctd/rows-without-index query-result)]
+     (is (= [["[true, false, null]"], ["[]"]] result)))))
+
 (deftest ^:parallel clickhouse-array-of-uint8
   (mt/test-driver
    :clickhouse
@@ -519,3 +536,16 @@
                   (data/run-mbql-query
                    sum_if_test_float
                    {:aggregation [[:sum-where $float_value [:= $discriminator "qaz"]]]}))))))))))
+
+(deftest ^:parallel clickhouse-array-inner-types
+  (mt/test-driver
+   :clickhouse
+   (is (= [["[a, b, c]"
+            "[null, d, e]"
+            "[1.0000, 2.0000, 3.0000]"
+            "[4.0000, null, 5.0000]"]]
+          (ctd/do-with-test-db
+           (fn [db]
+             (data/with-db db
+               (->> (data/run-mbql-query arrays_inner_types {})
+                    (mt/formatted-rows [str str str str])))))))))

test/metabase/driver/clickhouse_impersonation_test.clj

@@ -1,4 +1,4 @@
-(ns metabase.driver.clickhouse-test
+(ns metabase.driver.clickhouse-impersonation-test
   "SET ROLE (connection impersonation feature) tests on with single node or on-premise cluster setups."
   #_{:clj-kondo/ignore [:unsorted-required-namespaces]}
   (:require [clojure.test :refer :all]

test/metabase/driver/clickhouse_introspection_test.clj

@@ -1,4 +1,4 @@
-(ns metabase.driver.clickhouse-test
+(ns metabase.driver.clickhouse-introspection-test
   #_{:clj-kondo/ignore [:unsorted-required-namespaces]}
   (:require
    [clojure.test :refer :all]
@@ -13,6 +13,8 @@
    [metabase.test.data.interface :as tx]
    [toucan2.tools.with-temp :as t2.with-temp]))
 
+(use-fixtures :once ctd/create-test-db!)
+
 (defn- desc-table
   [table-name]
   (into #{} (map #(select-keys % [:name :database-type :base-type :database-required])

test/metabase/driver/clickhouse_substitution_test.clj

@@ -1,4 +1,4 @@
-(ns metabase.driver.clickhouse-test
+(ns metabase.driver.clickhouse-substitution-test
   #_{:clj-kondo/ignore [:unsorted-required-namespaces]}
   (:require [clojure.test :refer :all]
             [java-time.api :as t]

test/metabase/driver/clickhouse_temporal_bucketing_test.clj

@@ -1,4 +1,4 @@
-(ns metabase.driver.clickhouse-test
+(ns metabase.driver.clickhouse-temporal-bucketing-test
   #_{:clj-kondo/ignore [:unsorted-required-namespaces]}
   (:require
    [clojure.test :refer :all]
@@ -7,6 +7,8 @@
    [metabase.test.data :as data]
    [metabase.test.data.clickhouse :as ctd]))
 
+(use-fixtures :once ctd/create-test-db!)
+
 ;; See temporal_bucketing table definition
 ;; Fields values are (both in server and column timezones):
 ;; start_of_year == '2022-01-01 00:00:00'

test/metabase/driver/clickhouse_test.clj

@@ -5,11 +5,6 @@
             [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]
-            [metabase.driver.clickhouse-substitution-test]
-            [metabase.driver.clickhouse-temporal-bucketing-test]
             [metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
             [metabase.models.database :refer [Database]]
             [metabase.query-processor.compile :as qp.compile]

test/metabase/test/data/clickhouse.clj

@@ -31,7 +31,7 @@
    :ssl false
    :use_no_proxy false
    :use_server_time_zone_for_dates true
-   :product_name "metabase/1.50.1"
+   :product_name "metabase/1.50.2"
    :databaseTerm "schema"
    :remember_last_set_roles true
    :http_connection_provider "HTTP_URL_CONNECTION"

test/metabase/test/data/datasets.sql

@@ -245,3 +245,19 @@ DROP DATABASE IF EXISTS `Special@Characters~`;
 CREATE DATABASE `Special@Characters~`;
 DROP DATABASE IF EXISTS `'Escaping'`;
 CREATE DATABASE `'Escaping'`;
+
+-- arrays inner types test
+CREATE TABLE `metabase_test`.`arrays_inner_types`
+(
+    `arr_str`  Array(String),
+    `arr_nstr` Array(Nullable(String)),
+    `arr_dec`  Array(Decimal(18, 4)),
+    `arr_ndec` Array(Nullable(Decimal(18, 4)))
+)
+ENGINE Memory;
+INSERT INTO `metabase_test`.`arrays_inner_types` VALUES (
+    ['a', 'b', 'c'],
+    [NULL, 'd', 'e'],
+    [1, 2, 3],
+    [4, NULL, 5]
+);