Use `information_schema` as a basis for `describe-database` in Databricks driver (#48585) (#48708)

* Use information_schema for describe-database

* Unrelated fix: set dynamic-dataset-loading defaults to true

* cljfmt

* Fix test

Co-authored-by: lbrdnk <lbrdnk@users.noreply.github.com>

modules/drivers/databricks/src/metabase/driver/databricks.clj

@@ -10,6 +10,7 @@
    [metabase.driver.sql-jdbc.execute.legacy-impl :as sql-jdbc.legacy]
    [metabase.driver.sql-jdbc.sync :as sql-jdbc.sync]
    [metabase.driver.sql.query-processor :as sql.qp]
+   [metabase.driver.sync :as driver.s]
    [metabase.query-processor.timezone :as qp.timezone]
    [metabase.util :as u]
    [metabase.util.honey-sql-2 :as h2x]
@@ -44,6 +45,39 @@
     ((get-method sql-jdbc.sync/database-type->base-type :hive-like)
      driver database-type)))
 
+(defn- get-tables-sql
+  [catalog]
+  (assert (string? (not-empty catalog)))
+  [(str/join
+    "\n"
+    ["select"
+     "  TABLE_NAME as name,"
+     "  TABLE_SCHEMA as schema,"
+     "  COMMENT description"
+     "  from information_schema.tables"
+     "  where TABLE_CATALOG = ?"
+     "    AND TABLE_SCHEMA <> 'information_schema'"])
+   catalog])
+
+(defn- describe-database-tables
+  [database]
+  (let [[inclusion-patterns
+         exclusion-patterns] (driver.s/db-details->schema-filter-patterns database)
+        syncable? (fn [schema]
+                    (driver.s/include-schema? inclusion-patterns exclusion-patterns schema))]
+    (eduction
+     (filter (comp syncable? :schema))
+     (sql-jdbc.execute/reducible-query database (get-tables-sql (-> database :details :catalog))))))
+
+(defmethod driver/describe-database :databricks
+  [driver database]
+  (try
+    {:tables (into #{} (describe-database-tables database))}
+    (catch Throwable e
+      (throw (ex-info (format "Error in %s describe-database: %s" driver (ex-message e))
+                      {}
+                      e)))))
+
 (defmethod sql-jdbc.sync/describe-fields-sql :databricks
   [driver & {:keys [schema-names table-names]}]
   (sql/format {:select [[:c.column_name :name]

modules/drivers/databricks/test/metabase/driver/databricks_test.clj

@@ -8,10 +8,11 @@
    [metabase.test :as mt]
    [toucan2.core :as t2]))
 
+;; Because the datasets that are tested are preloaded, it is fine just to modify the database details to sync other schemas.
 (deftest ^:parallel sync-test
-  (testing "`driver/describe-database` implementation returns expected resutls."
-    (mt/test-driver
-      :databricks
+  (mt/test-driver
+    :databricks
+    (testing "`driver/describe-database` implementation returns expected results for inclusion of test-data schema."
       (is (= {:tables
               #{{:name "venues", :schema "test-data", :description nil}
                 {:name "checkins", :schema "test-data", :description nil}
@@ -21,7 +22,27 @@
                 {:name "reviews", :schema "test-data", :description nil}
                 {:name "orders", :schema "test-data", :description nil}
                 {:name "products", :schema "test-data", :description nil}}}
-             (driver/describe-database :databricks (mt/db)))))))
+             (driver/describe-database :databricks (mt/db)))))
+    (testing "`driver/describe-database` returns expected results for `all` schema filters."
+      (let [actual-tables (driver/describe-database :databricks (-> (mt/db)
+                                                                    (update :details dissoc :schema-filters-patterns)
+                                                                    (update :details assoc  :schema-filters-type "all")))]
+        (testing "tables from multiple schemas were found"
+          (is (contains? (:tables actual-tables) {:name "venues", :schema "test-data", :description nil}))
+          (is (contains? (:tables actual-tables) {:name "checkins", :schema "test-data", :description nil}))
+          (is (contains? (:tables actual-tables) {:name "airport", :schema "airports", :description nil}))
+          (is (contains? (:tables actual-tables) {:name "bird", :schema "bird-flocks", :description nil})))
+        (testing "information_schema is excluded"
+          (is (empty? (filter #(= "information_schema" (:schema %)) (:tables actual-tables)))))))
+    (testing "`driver/describe-database` returns expected results for `exclusion` schema filters."
+      (let [actual-tables (driver/describe-database :databricks (update (mt/db) :details assoc
+                                                                        :schema-filters-patterns "test-data"
+                                                                        :schema-filters-type "exclusion"))]
+        (testing "tables from multiple schemas were found"
+          (is (not (contains? (:tables actual-tables) {:name "venues", :schema "test-data", :description nil})))
+          (is (not (contains? (:tables actual-tables) {:name "checkins", :schema "test-data", :description nil})))
+          (is (contains? (:tables actual-tables) {:name "airport", :schema "airports", :description nil}))
+          (is (contains? (:tables actual-tables) {:name "bird", :schema "bird-flocks", :description nil})))))))
 
 (deftest ^:parallel describe-fields-test
   (testing "`describe-fields` returns expected values"

modules/drivers/databricks/test/metabase/test/data/databricks.clj

@@ -126,7 +126,7 @@
   [driver dbdef]
   (if *allow-database-deletion*
     ((get-method tx/destroy-db! :sql-jdbc/test-extensions) driver dbdef)
-    (log/warn "`*allow-database-creation*` is `false`. Database removal is suppressed.")))
+    (log/warn "`*allow-database-deletion*` is `false`. Database removal is suppressed.")))
 
 ;; Differences to the :sql-jdbc/test-extensions original: false transactions, not using `jdbc/execute!` for
 ;; timezone setting, not overriding database timezone.

src/metabase/driver.clj

@@ -735,7 +735,8 @@
                               :schemas                                true
                               :test/jvm-timezone-setting              true
                               :fingerprint                            true
-                              :upload-with-auto-pk                    true}]
+                              :upload-with-auto-pk                    true
+                              :test/dynamic-dataset-loading           true}]
   (defmethod database-supports? [::driver feature] [_driver _feature _db] supported?))
 
 ;;; By default a driver supports `:native-parameter-card-reference` if it supports `:native-parameters` AND