Skip to content

Commit

Permalink
Fix incorrect UInt8 arrays serialization (#211)
Browse files Browse the repository at this point in the history
  • Loading branch information
slvrtrn authored Dec 21, 2023
1 parent 973abb7 commit 77b9082
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 10 deletions.
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
# 1.3.1

### Bug fixes
* Fixed incorrect serialization of `Array(UInt8)` columns ([#209](https://github.com/ClickHouse/metabase-clickhouse-driver/issues/209))

# 1.3.0

### New features
Expand All @@ -11,7 +16,7 @@
Metabase 0.47.7+ only.

### New features
* Added [datetimeDiff](https://www.metabase.com/docs/latest/questions/query-builder/expressions/datetimediff) function support. See ([#117](https://github.com/ClickHouse/metabase-clickhouse-driver/issues/117)).
* Added [datetimeDiff](https://www.metabase.com/docs/latest/questions/query-builder/expressions/datetimediff) function support ([#117](https://github.com/ClickHouse/metabase-clickhouse-driver/issues/117))

# 1.2.4

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ docker run -d -p 3000:3000 \
| 0.46.x | 1.1.7 |
| 0.47.x | 1.2.3 |
| 0.47.7+ | 1.2.5 |
| 0.48.0-RC1 | 1.3.0 |
| 0.48.x | 1.3.1 |

## Creating a Metabase Docker image with ClickHouse driver

Expand Down
1 change: 1 addition & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ services:
container_name: metabase-with-clickhouse-driver
environment:
'MB_HTTP_TIMEOUT': '5000'
'JAVA_TIMEZONE': 'UTC'
ports:
- '3000:3000'
volumes:
Expand Down
2 changes: 1 addition & 1 deletion resources/metabase-plugin.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
info:
name: Metabase ClickHouse Driver
version: 1.3.0
version: 1.3.1
description: Allows Metabase to connect to ClickHouse databases.
contact-info:
name: ClickHouse
Expand Down
2 changes: 1 addition & 1 deletion src/metabase/driver/clickhouse.clj
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
(driver/register! :clickhouse :parent :sql-jdbc)

(defmethod driver/display-name :clickhouse [_] "ClickHouse")
(def ^:private product-name "metabase/1.3.0")
(def ^:private product-name "metabase/1.3.1")

(defmethod driver/prettify-native-form :clickhouse [_ native-form]
(sql.u/format-sql-and-fix-params :mysql native-form))
Expand Down
21 changes: 16 additions & 5 deletions src/metabase/driver/clickhouse_qp.clj
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
[metabase.driver.sql.util.unprepare :as unprepare]
[metabase.mbql.util :as mbql.u]
[metabase.util.date-2 :as u.date]
[metabase.util.honey-sql-2 :as h2x])
[metabase.util.honey-sql-2 :as h2x]
[metabase.util.log :as log])
(:import [com.clickhouse.data.value ClickHouseArrayValue]
[java.sql ResultSet ResultSetMetaData Types]
[java.time
Expand Down Expand Up @@ -393,14 +394,24 @@
(.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 _ ^Integer i]
[_ ^ResultSet rs ^ResultSetMetaData rsmeta ^Integer i]
(fn []
(when-let [arr (.getArray rs i)]
(let [inner (.getArray arr)]
(let [array-base-type (get-array-base-type rsmeta i)
inner (.getArray arr)]
(cond
;; Booleans are returned as just bytes
(bytes? inner)
(= array-base-type "Bool")
(str "[" (str/join ", " (map #(if (= 1 %) "true" "false") inner)) "]")
;; All other primitives
(.isPrimitive (.getComponentType (.getClass inner)))
Expand Down
15 changes: 15 additions & 0 deletions test/metabase/driver/clickhouse_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,21 @@
result (ctd/rows-without-index query-result)]
(is (= [["[true, false, true]"], ["[]"]] result)))))

(deftest clickhouse-array-of-uint8
(mt/test-driver
:clickhouse
(let [row1 (into-array (list 42 100 2))
row2 (into-array nil)
query-result (data/dataset
(tx/dataset-definition "metabase_tests_array_of_uint8"
["test-data-array-of-uint8"
[{:field-name "my_array_of_uint8"
:base-type {:native "Array(UInt8)"}}]
[[row1] [row2]]])
(data/run-mbql-query test-data-array-of-uint8 {}))
result (ctd/rows-without-index query-result)]
(is (= [["[42, 100, 2]"], ["[]"]] result)))))

(deftest clickhouse-array-of-floats
(mt/test-driver
:clickhouse
Expand Down
2 changes: 1 addition & 1 deletion test/metabase/test/data/clickhouse.clj
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
:ssl false
:use_no_proxy false
:use_server_time_zone_for_dates true
:product_name "metabase/1.3.0"})
:product_name "metabase/1.3.1"})

(defn rows-without-index
"Remove the Metabase index which is the first column in the result set"
Expand Down

0 comments on commit 77b9082

Please sign in to comment.