perf: Implement faster sync methods for mysql (#49011)

* perf: Implement faster sync methods for postgres

Fixes #48575

Pulls work from redshift into the common postgres driver.

* Fix tests and formatting

* Move nested-field-column sync to sync functions so describe-fields will also get them

* Fix test

* Fix test

* Remove fixed safety test

* Add test specific database-supports feature for pk metadata

* Fix test

* perf: faster mysql sync with describe-fields

Fixes: #49010

* Adrress PR feedback

* Fix tests

* Fix test

* Add nil table-schema

* Don't use subselect for field-comment

* Fix quoting weird identifiers

* Make format string inline

* Update src/metabase/driver/mysql.clj

Co-authored-by: metamben <103100869+metamben@users.noreply.github.com>

* Update src/metabase/driver/mysql.clj

Co-authored-by: metamben <103100869+metamben@users.noreply.github.com>

* Fix tests

* Fix database-type

* Fix tests

* Fix test

* Fix tests

* Exclude mysql table_schema

* Handle tinyInt1IsBit

* Fix test

* Only get one db at a time

---------

Co-authored-by: metamben <103100869+metamben@users.noreply.github.com>

src/metabase/driver/mysql.clj

@@ -58,6 +58,8 @@
                               ;; server and the columns themselves. Since this isn't something we can really change in
                               ;; the query itself don't present the option to the users in the UI
                               :case-sensitivity-string-filter-options false
+                              :describe-fields                        true
+                              :describe-fks                           true
                               :convert-timezone                       true
                               :datetime-diff                          true
                               :full-join                              false
@@ -917,3 +919,57 @@
                   (when-let [privileges (not-empty (set/union all-table-privileges (get table-privileges table-name)))]
                     [table-name privileges])))
           table-names)))
+
+;; Describe the Fields present in a `table`. This just hands off to the normal SQL driver implementation of the same
+;; name, but coerces boolean fields since mysql returns them as 0/1 integers
+(defmethod driver/describe-fields :mysql
+  [driver database & args]
+  (eduction
+   (map (fn [col]
+          (-> col
+              (update :pk? pos?)
+              (update :database-position int) ;; Comes in as biginteger
+              (update :database-required pos?)
+              (update :database-is-auto-increment pos?))))
+   (apply (get-method driver/describe-fields :sql-jdbc) driver database args)))
+
+(defmethod sql-jdbc.sync/describe-fields-sql :mysql
+  [driver & {:keys [table-names details]}]
+  (sql/format {:select [[:c.column_name :name]
+                        [[:- :c.ordinal_position 1] :database-position]
+                        [nil :table-schema]
+                        [:c.table_name :table-name]
+                        (if (some-> details :additional-options (str/includes? "tinyInt1isBit=false"))
+                          [[:upper :c.data_type] :database-type]
+                          [[:if [:= :column_type [:inline "tinyint(1)"]] [:inline "BIT"] [:upper :c.data_type]] :database-type])
+                        [[:= :c.extra [:inline "auto_increment"]] :database-is-auto-increment]
+                        [[:and
+                          [:or [:= :column_default nil] [:= [:lower :column_default] [:inline "null"]]]
+                          [:= :is_nullable [:inline "NO"]]
+                          [:not [:= :c.extra [:inline "auto_increment"]]]]
+                         :database-required]
+                        [[:= :c.column_key [:inline "PRI"]] :pk?]
+                        [[:nullif :c.column_comment [:inline ""]] :field-comment]]
+               :from [[:information_schema.columns :c]]
+               :where
+               [:and [:raw "c.table_schema not in ('information_schema','performance_schema','sys','mysql')"]
+                [:raw "c.table_name not in ('innodb_table_stats', 'innodb_index_stats')"]
+                (when (:db details) [:= :c.table_schema (:db details)])
+                (when (seq table-names) [:in [:lower :c.table_name] (map u/lower-case-en table-names)])]}
+              :dialect (sql.qp/quote-style driver)))
+
+(defmethod sql-jdbc.sync/describe-fks-sql :mysql
+  [driver & {:keys [table-names]}]
+  (sql/format {:select [[nil :pk-table-schema]
+                        [:a.referenced_table_name :pk-table-name]
+                        [:a.referenced_column_name :pk-column-name]
+                        [nil :fk-table-schema]
+                        [:a.table_name :fk-table-name]
+                        [:a.column_name :fk-column-name]]
+               :from [[:information_schema.key_column_usage :a]]
+               :join [[:information_schema.table_constraints :b] [:using :constraint_schema :constraint_name :table_name]]
+               :where [:and [:= :b.constraint_type [:inline "FOREIGN KEY"]]
+                       [:!= :a.referenced_table_schema nil]
+                       (when (seq table-names) [:in :a.table_name table-names])]
+               :order-by [:a.table_name]}
+              :dialect (sql.qp/quote-style driver)))

src/metabase/driver/sql_jdbc/sync/describe_table.clj

@@ -179,8 +179,7 @@
                semantic-type  (calculated-semantic-type driver (:name col) (:database-type col))
                json?          (isa? base-type :type/JSON)]
            (merge
-            (u/select-non-nil-keys col [:table-schema
-                                        :table-name
+            (u/select-non-nil-keys col [:table-name
                                         :pk?
                                         :name
                                         :database-type
@@ -188,7 +187,8 @@
                                         :field-comment
                                         :database-required
                                         :database-is-auto-increment])
-            {:base-type         base-type
+            {:table-schema      (:table-schema col) ;; can be nil
+             :base-type         base-type
              ;; json-unfolding is true by default for JSON fields, but this can be overridden at the DB level
              :json-unfolding    json?}
             (when semantic-type
@@ -201,7 +201,7 @@
   [driver db]
   (comp
    (describe-fields-xf driver db)
-   (map-indexed (fn [i col] (assoc col :database-position i)))))
+   (map-indexed (fn [i col] (dissoc (assoc col :database-position i) :table-schema)))))
 
 (defmulti describe-table-fields
   "Returns a set of column metadata for `table` using JDBC Connection `conn`."
@@ -298,7 +298,7 @@
   "Returns a SQL query ([sql & params]) for use in the default JDBC implementation of [[metabase.driver/describe-fields]],
  i.e. [[describe-fields]]."
   {:added    "0.49.1"
-   :arglists '([driver & {:keys [schema-names table-names]}])}
+   :arglists '([driver & {:keys [schema-names table-names details]}])}
   driver/dispatch-on-initialized-driver
   :hierarchy #'driver/hierarchy)
 
@@ -310,7 +310,7 @@
     []
     (eduction
      (describe-fields-xf driver db)
-     (sql-jdbc.execute/reducible-query db (describe-fields-sql driver args)))))
+     (sql-jdbc.execute/reducible-query db (describe-fields-sql driver (assoc args :details (:details db)))))))
 
 (defn- describe-table-fks*
   [_driver ^Connection conn {^String schema :schema, ^String table-name :name} & [^String db-name-or-nil]]

test/metabase/driver/sql_jdbc/sync/describe_table_test.clj

@@ -5,6 +5,7 @@
    [clojure.set :as set]
    [clojure.string :as str]
    [clojure.test :refer :all]
+   [medley.core :as m]
    [metabase.db.metadata-queries :as metadata-queries]
    [metabase.driver :as driver]
    [metabase.driver.mysql :as mysql]
@@ -137,10 +138,11 @@
   (let [driver (driver.u/database->driver db)]
     (sort-by :database-position
              (if (driver.u/supports? driver :describe-fields db)
-               (vec (driver/describe-fields driver
-                                            db
-                                            :schema-names [(:schema table)]
-                                            :table-names [(:name table)]))
+               (vec (m/mapply driver/describe-fields
+                              driver
+                              db
+                              (cond-> {:table-names [(:name table)]}
+                                (:schema table) (assoc :schema-names [(:schema table)]))))
                (:fields (driver/describe-table driver db table))))))
 
 (defmethod driver/database-supports? [::driver/driver ::describe-pks]
@@ -166,7 +168,10 @@
             [5 false false false]]
            (sort-by
             :first
-            (map (juxt :database-position :database-required :database-is-auto-increment (comp boolean :pk?))
+            (map (juxt :database-position
+                       :database-required
+                       :database-is-auto-increment
+                       (comp boolean :pk?))
                  (describe-fields-for-table (mt/db) (t2/select-one Table :id (mt/id :venues))))))))
     (mt/test-drivers (mt/normal-drivers-without-feature :actions)
       (is (=?

test/metabase/driver_test.clj

@@ -211,9 +211,10 @@
 (defn- describe-fields-for-table [db table]
   (sort-by :database-position
            (if (driver/database-supports? driver/*driver* :describe-fields db)
-             (vec (driver/describe-fields driver/*driver* db
-                                          :schema-names [(:schema table)]
-                                          :table-names [(:name table)]))
+             (mapv #(dissoc % :table-name :table-schema)
+                   (driver/describe-fields driver/*driver* db
+                                           :schema-names [(:schema table)]
+                                           :table-names [(:name table)]))
              (:fields (driver/describe-table driver/*driver* db table)))))
 
 (deftest ^:parallel describe-fields-or-table-test