Skip to content

Commit

Permalink
Add 0.47.7 question -> SQL conversion fix (#206)
Browse files Browse the repository at this point in the history
* Add 0.47.7 question -> SQL conversion fix

* Fix CI
  • Loading branch information
slvrtrn authored Nov 20, 2023
1 parent 6377168 commit 6089b7f
Show file tree
Hide file tree
Showing 8 changed files with 22 additions and 13 deletions.
3 changes: 1 addition & 2 deletions .github/workflows/check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ jobs:
uses: actions/checkout@v2
with:
repository: metabase/metabase
# The latest commit on release-0.47.x branch - fixed failing MB tests
ref: dba05c0b6f6cbc6e2f8247c03cc5fcc986cd3fe5
ref: v0.47.8

- name: Checkout Driver Repo
uses: actions/checkout@v2
Expand Down
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
# 1.2.4

Metabase 0.47.7+ only.

### Bug fixes
* Fixed UI question -> SQL conversion creating incorrect queries due to superfluous spaces in columns/tables/database names.

# 1.2.3

### Bug fixes
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ docker run -d -p 3000:3000 \
| 0.45.x | 1.1.0 |
| 0.46.x | 1.1.7 |
| 0.47.x | 1.2.3 |
| 0.47.7+ | 1.2.4 |

## Creating a Metabase Docker image with ClickHouse driver

Expand Down
2 changes: 1 addition & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ services:
hostname: server.clickhouseconnect.test

metabase:
image: metabase/metabase:v0.47.2
image: metabase/metabase:v0.47.8
container_name: metabase-with-clickhouse-driver
environment:
'MB_HTTP_TIMEOUT': '5000'
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.2.3
version: 1.2.4
description: Allows Metabase to connect to ClickHouse databases.
contact-info:
name: ClickHouse
Expand Down
9 changes: 7 additions & 2 deletions src/metabase/driver/clickhouse.clj
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
(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]]
[metabase.driver :as driver]
[metabase.driver.clickhouse-introspection]
[metabase.driver.clickhouse-nippy]
Expand All @@ -11,14 +13,17 @@
[metabase.driver.sql-jdbc [common :as sql-jdbc.common]
[connection :as sql-jdbc.conn]
[sync :as sql-jdbc.sync]]
[metabase [config :as config]]))
[metabase.driver.sql.util :as sql.u]))

(set! *warn-on-reflection* true)

(driver/register! :clickhouse :parent :sql-jdbc)

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

(defmethod driver/prettify-native-form :clickhouse [_ native-form]
(sql.u/format-sql-and-fix-params :mysql native-form))

(doseq [[feature supported?] {:standard-deviation-aggregations true
:foreign-keys (not config/is-test?)
Expand Down
9 changes: 3 additions & 6 deletions test/metabase/driver/clickhouse_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -724,11 +724,8 @@
:clickhouse
(let [query (data/mbql-query venues {:fields [$id] :order-by [[:asc $id]] :limit 5})
{compiled :query} (qp/compile-and-splice-parameters query)
_pretty (mdb.query/format-sql compiled :clickhouse)]
pretty (driver/prettify-native-form :clickhouse compiled)]
(testing "compiled"
(is (= "SELECT `test_data`.`venues`.`id` AS `id` FROM `test_data`.`venues` ORDER BY `test_data`.`venues`.`id` ASC LIMIT 5" compiled)))
;; Ignored due to Metabase bug, see https://github.com/metabase/metabase/issues/34235
;; FIXME: uncomment once it is resolved
;; (testing "pretty"
;; (is (= "SELECT\n `test_data`.`venues`.`id` AS `id`\nFROM `test_data`.`venues`\nORDER BY\n `test_data`.`venues`.`id` ASC\nLIMIT\n 5" pretty)))
)))
(testing "pretty"
(is (= "SELECT\n `test_data`.`venues`.`id` AS `id`\nFROM\n `test_data`.`venues`\nORDER BY\n `test_data`.`venues`.`id` ASC\nLIMIT\n 5" pretty))))))
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.2.3"})
:product_name "metabase/1.2.4"})

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

0 comments on commit 6089b7f

Please sign in to comment.