Specify multiple databases on connect (#152)

* Add multiple-schema feature Co-authored-by: veschin <veschindev@gmail.com> Co-authored-by: oveshchin <oveshchin@phoenixit.ru> * Refactor multiple schemas & minor fixes Co-authored-by: veschin <veschindev@gmail.com> Co-authored-by: oveshchin <oveshchin@phoenixit.ru> --------- Co-authored-by: Oleg Veschin <oveshchin@phoenixit.ru>
ClickHouse · Mar 23, 2023 · 91a3857 · 91a3857
1 parent 2f8135a
commit 91a3857
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,9 @@
+# 1.1.4
+
+### New features 
+
+* Database name can now contain multiple schemas in UI field, which tells the driver to scan selected databases. (separator can be set in `metabase.driver.clickhouse/SEPARATOR`) 
+
 # 1.1.2
 
 ### Bug fixes

diff --git a/resources/metabase-plugin.yaml b/resources/metabase-plugin.yaml
@@ -18,7 +18,7 @@ driver:
     - user
     - password
     - name: dbname
-      display-name: Database Name
+      display-name: Databases Names
       placeholder: default
     - name: scan-all-databases
       display-name: Scan all databases

diff --git a/src/metabase/driver/clickhouse.clj b/src/metabase/driver/clickhouse.clj
@@ -117,41 +117,52 @@
               "%"            ; tablePattern "%" = match all tables
               allowed-table-types))
 
-(defn- get-tables-in-db
-  [^DatabaseMetaData metadata db-name]
-  ;; maybe snake-case is unnecessary here
-  (let [db-name-snake-case (ddl.i/format-name :clickhouse (or db-name "default"))]
-    (tables-set
-     (vec (jdbc/metadata-result
-           (get-tables-from-metadata metadata db-name-snake-case))))))
-
-(defn- get-all-tables
-  [metadata]
-  (tables-set
-   (filter
-    #(not (contains? excluded-schemas (get % :table_schem)))
-    (vec (jdbc/metadata-result
-          (get-tables-from-metadata metadata "%"))))))
-
 (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 #(->> (get % :table_schem)
+                       (contains? excluded-schemas)
+                       (not)))
+         (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])))
 
+;; NOTE: option for collection tables from many schemas
+(def ^:private SEPARATOR #" ")
+(defn- get-multiple-tables [db]
+  (->> (for [schema (as-> (or (get-db-name db) "default") schemas
+                      (str/split schemas SEPARATOR)
+                      (remove empty? schemas)
+                      (map (comp #(ddl.i/format-name :clickhouse %) str/trim) schemas))]
+         (jdbc/with-db-metadata [metadata (->spec db)]
+           (jdbc/metadata-result
+            (get-tables-from-metadata metadata schema))))
+       (apply concat)
+       (tables-set)))
+
 (defmethod driver/describe-database :clickhouse
-  [_ db]
-  (jdbc/with-db-metadata [metadata (->spec db)]
-    (let [tables (if (get-in db [:details :scan-all-databases])
-                   (get-all-tables metadata)
-                   (get-tables-in-db metadata (get-db-name db)))]
-      {:tables tables})))
+  [_ {{:keys [scan-all-databases]}
+      :details :as db}]
+  {:tables
+   (cond
+     scan-all-databases
+     (get-all-tables db)
+
+     :selected-schemas
+     (get-multiple-tables db))})
 
 (defmethod driver/describe-table :clickhouse
   [_ database table]

diff --git a/test/metabase/driver/clickhouse_test.clj b/test/metabase/driver/clickhouse_test.clj
@@ -566,8 +566,8 @@
           (is (=
                {:tables
                 #{agg-fn-table boolean-table enum-table ipaddress-table}}
-               describe-result))))
-      (testing "scanning all databases"
+               describe-result)))))
+    (testing "scanning all databases"
         (mt/with-temp Database
           [db {:engine :clickhouse
                :details {:dbname "default"
@@ -588,4 +588,26 @@
             (is (not (some #(= (get % :schema) "information_schema")
                            (:tables describe-result))))
             (is (not (some #(= (get % :schema) "INFORMATION_SCHEMA")
-                           (:tables describe-result))))))))))
+                           (:tables describe-result)))))))
+    (testing "scanning multiple databases"
+      (mt/with-temp Database
+        [db {:engine :clickhouse
+             :details {:dbname "metabase_test information_schema"}}]
+        (let [{:keys [tables] :as _describe-result}
+              (driver/describe-database :clickhouse db)
+              tables-table  {:name        "tables"
+                             :description nil
+                             :schema      "information_schema"}
+              columns-table {:name        "columns"
+                             :description nil
+                             :schema      "information_schema"}]
+
+          ;; NOTE: tables from `metabase_test`
+          (is (contains? tables agg-fn-table))
+          (is (contains? tables boolean-table))
+          (is (contains? tables enum-table))
+          (is (contains? tables ipaddress-table))
+
+          ;; NOTE: tables from `information_schema`
+          (is (contains? tables tables-table))
+          (is (contains? tables columns-table)))))))