Sync index info

docs/developers-guide/driver-changelog.md

@@ -4,6 +4,10 @@ title: Driver interface changelog
 
 # Driver Interface Changelog
 
+## Metabase 0.49.0
+- A new driver method has been added [[driver/describe-table-indexes]] along with a new feature `:index-info`.
+  This method is used to get a set of column names that are indexed or are the first columns in a composite index.
+
 ## Metabase 0.48.0
 
 - The MBQL schema in `metabase.mbql.schema` now uses [Malli](https://github.com/metosin/malli) instead of

resources/migrations/001_update_migrations.yaml

@@ -4186,6 +4186,21 @@ databaseChangeLog:
                     nullable: true
                   remarks: 'Hash of normalized query, calculated in middleware.cache'
 
+  - changeSet:
+      id: v49.00-003
+      author: qnkhuat
+      comment: 'Adds query_execution.cache_hash -> query_cache.query_hash'
+      changes:
+        - addColumn:
+            tableName: metabase_field
+            columns:
+              - column:
+                  name: database_indexed
+                  type: boolean
+                  constraints:
+                    nullable: true
+                  remarks: 'If the database supports indexing, this column indicate whether or not a field is indexed, or is the 1st column in a composite index'
+
   # >>>>>>>>>> DO NOT ADD NEW MIGRATIONS BELOW THIS LINE! ADD THEM ABOVE <<<<<<<<<<
 
 

src/metabase/driver.clj

@@ -316,6 +316,12 @@
   dispatch-on-initialized-driver
   :hierarchy #'hierarchy)
 
+(defmulti describe-table-indexes
+  "Returns a set of field names that are either indexed or are the first columns in a composite index."
+  {:added "0.49.0" :arglists '([driver database table])}
+  dispatch-on-initialized-driver
+  :hierarchy #'hierarchy)
+
 (defmulti escape-entity-name-for-metadata
   "escaping for when calling `.getColumns` or `.getTables` on table names or schema names. Useful for when a database
   driver has difference escaping rules for table or schema names when used from metadata.
@@ -532,7 +538,10 @@
     :connection-impersonation-requires-role
 
     ;; Does the driver require specifying a collection (table) for native queries? (mongo)
-    :native-requires-specified-collection})
+    :native-requires-specified-collection
+
+    ;; Does the driver support column(s) support storing index info
+    :index-info})
 
 
 (defmulti supports?

src/metabase/driver/h2.clj

@@ -78,7 +78,8 @@
                               :datetime-diff             true
                               :now                       true
                               :test/jvm-timezone-setting false
-                              :uploads                   true}]
+                              :uploads                   true
+                              :index-info                true}]
   (defmethod driver/database-supports? [:h2 feature]
     [_driver _feature _database]
     supported?))

src/metabase/driver/mysql.clj

@@ -54,19 +54,20 @@
 
 (defmethod driver/display-name :mysql [_] "MySQL")
 
-(doseq [[feature supported?] {:persist-models          true
-                              :convert-timezone        true
-                              :datetime-diff           true
-                              :now                     true
-                              :regex                   false
-                              :percentile-aggregations false
-                              :full-join               false
-                              :uploads                 true
-                              :schemas                 false
+(doseq [[feature supported?] {:persist-models                         true
+                              :convert-timezone                       true
+                              :datetime-diff                          true
+                              :now                                    true
+                              :regex                                  false
+                              :percentile-aggregations                false
+                              :full-join                              false
+                              :uploads                                true
+                              :schemas                                false
                               ;; MySQL LIKE clauses are case-sensitive or not based on whether the collation of the 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}]
+                              :case-sensitivity-string-filter-options false
+                              :index-info                             true}]
   (defmethod driver/database-supports? [:mysql feature] [_driver _feature _db] supported?))
 
 ;; This is a bit of a lie since the JSON type was introduced for MySQL since 5.7.8.
@@ -672,7 +673,7 @@
   to calculate one by hand."
   [driver database ^OffsetDateTime offset-time]
   (let [zone-id (t/zone-id (driver/db-default-timezone driver database))]
-    (t/local-date-time offset-time zone-id )))
+    (t/local-date-time offset-time zone-id)))
 
 (defmulti ^:private value->string
   "Convert a value into a string that's safe for insertion"

src/metabase/driver/postgres.clj

@@ -54,6 +54,9 @@
 
 (driver/register! :postgres, :parent :sql-jdbc)
 
+(defmethod driver/display-name :postgres [_] "PostgreSQL")
+
+;; Features that are supported by Postgres and all of its child drivers like Redshift
 (doseq [[feature supported?] {:convert-timezone         true
                               :datetime-diff            true
                               :now                      true
@@ -66,19 +69,14 @@
   [_driver _feat db]
   (driver.common/json-unfolding-default db))
 
-;;; +----------------------------------------------------------------------------------------------------------------+
-;;; |                                             metabase.driver impls                                              |
-;;; +----------------------------------------------------------------------------------------------------------------+
-
-(defmethod driver/display-name :postgres [_] "PostgreSQL")
-
+;; Features that are supported by postgres only
 (doseq [feature [:actions
                  :actions/custom
                  :table-privileges
-                 :uploads]]
+                 :uploads
+                 :index-info]]
   (defmethod driver/database-supports? [:postgres feature]
     [driver _feat _db]
-    ;; only supported for Postgres for right now. Not supported for child drivers like Redshift or whatever.
     (= driver :postgres)))
 
 ;;; +----------------------------------------------------------------------------------------------------------------+

src/metabase/driver/sql_jdbc.clj

@@ -101,6 +101,10 @@
   [driver database table]
   (sql-jdbc.sync/describe-table-fks driver database table))
 
+(defmethod driver/describe-table-indexes :sql-jdbc
+  [driver database table]
+  (sql-jdbc.sync/describe-table-indexes driver database table))
+
 (defmethod sql.qp/cast-temporal-string [:sql-jdbc :Coercion/ISO8601->DateTime]
   [_driver _semantic_type expr]
   (hx/->timestamp expr))

src/metabase/driver/sql_jdbc/sync.clj

@@ -29,6 +29,7 @@
   describe-table
   describe-table-fields
   describe-table-fks
+  describe-table-indexes
   get-catalogs
   pattern-based-database-type->base-type]
 

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

@@ -274,6 +274,35 @@
      (fn [^Connection conn]
        (describe-table-fks* driver conn table db-name-or-nil)))))
 
+(defn describe-table-indexes
+  "Default implementation of [[metabase.driver/describe-table-indexes]] for SQL JDBC drivers. Uses JDBC DatabaseMetaData."
+  [driver db table]
+  (sql-jdbc.execute/do-with-connection-with-options
+   driver
+   db
+   nil
+   (fn [^Connection conn]
+     ;; https://docs.oracle.com/javase/8/docs/api/java/sql/DatabaseMetaData.html#getIndexInfo-java.lang.String-java.lang.String-java.lang.String-boolean-boolean-
+     (with-open [index-info-rs (.getIndexInfo (.getMetaData conn)
+                                              nil ;; catalog
+                                              (:schema table)
+                                              (:name table)
+                                              ;; when true, return only indices for unique values when
+                                              ;; false, return indices regardless of whether unique or not
+                                              false
+                                              ;; when true, result is allowed to reflect approximate or out of data
+                                              ;; values. when false, results are requested to be accurate
+                                              false)]
+      (-> (group-by :index_name (into []
+                                      ;; filtered indexes are ignored
+                                      (filter #(nil? (:filter_condition %)))
+                                      (jdbc/reducible-result-set index-info-rs {})))
+          (update-vals (fn [idx-values]
+                         ;; we only sync columns that are either singlely indexed or is the first key in a composite index
+                         (first (map :column_name (sort-by :ordinal_position idx-values)))))
+          vals
+          set)))))
+
 (def ^:dynamic *nested-field-column-max-row-length*
   "Max string length for a row for nested field column before we just give up on parsing it.
   Marked as mutable because we mutate it for tests."

src/metabase/sync/fetch_metadata.clj

@@ -6,6 +6,7 @@
    [metabase.driver :as driver]
    [metabase.driver.sql-jdbc.sync :as sql-jdbc.sync]
    [metabase.driver.util :as driver.u]
+   [metabase.lib.schema.common :as lib.schema.common]
    [metabase.sync.interface :as i]
    [metabase.util.malli :as mu]))
 
@@ -35,3 +36,9 @@
   (let [driver (driver.u/database->driver database)]
     (when (driver/database-supports? driver :nested-field-columns database)
       (sql-jdbc.sync/describe-nested-field-columns driver database table))))
+
+(mu/defn index-metadata :- [:maybe [:set ::lib.schema.common/non-blank-string]]
+  "Get information about the indexes belonging to `table`."
+  [database :- i/DatabaseInstance
+   table    :- i/TableInstance]
+  (driver/describe-table-indexes (driver.u/database->driver database) database table))

src/metabase/sync/sync_metadata.clj

@@ -7,11 +7,13 @@
    3.  Sync FKs    (`metabase.sync.sync-metadata.fks`)
    4.  Sync Metabase Metadata table (`metabase.sync.sync-metadata.metabase-metadata`)"
   (:require
+   [metabase.models.table :as table]
    [metabase.sync.fetch-metadata :as fetch-metadata]
    [metabase.sync.interface :as i]
    [metabase.sync.sync-metadata.dbms-version :as sync-dbms-ver]
    [metabase.sync.sync-metadata.fields :as sync-fields]
    [metabase.sync.sync-metadata.fks :as sync-fks]
+   [metabase.sync.sync-metadata.indexes :as sync-indexes]
    [metabase.sync.sync-metadata.metabase-metadata :as metabase-metadata]
    [metabase.sync.sync-metadata.sync-table-privileges :as sync-table-privileges]
    [metabase.sync.sync-metadata.sync-timezone :as sync-tz]
@@ -40,6 +42,10 @@
   (format "Total number of foreign keys sync''d %d, %d updated and %d tables failed to update"
           total-fks updated-fks total-failed))
 
+(defn- sync-indexes-summary [{:keys [total-indexes added-indexes removed-indexes]}]
+  (format "Total number of indexes sync''d %d, %d added and %d removed"
+          total-indexes added-indexes removed-indexes))
+
 (defn- make-sync-steps [db-metadata]
   [(sync-util/create-sync-step "sync-dbms-version" sync-dbms-ver/sync-dbms-version! sync-dbms-version-summary)
    (sync-util/create-sync-step "sync-timezone" sync-tz/sync-timezone! sync-timezone-summary)
@@ -49,6 +55,8 @@
    (sync-util/create-sync-step "sync-fields" sync-fields/sync-fields! sync-fields-summary)
    ;; Now for each table, sync the FKS. This has to be done after syncing all the fields to make sure target fields exist
    (sync-util/create-sync-step "sync-fks" sync-fks/sync-fks! sync-fks-summary)
+   ;; Sync index info if the database supports it
+   (sync-util/create-sync-step "sync-indexes" sync-indexes/maybe-sync-indexes! sync-indexes-summary)
    ;; finally, sync the metadata metadata table if it exists.
    (sync-util/create-sync-step "sync-metabase-metadata" #(metabase-metadata/sync-metabase-metadata! % db-metadata))
    ;; Now sync the table privileges
@@ -67,5 +75,7 @@
 (mu/defn sync-table-metadata!
   "Sync the metadata for an individual `table` -- make sure Fields and FKs are up-to-date."
   [table :- i/TableInstance]
-  (sync-fields/sync-fields-for-table! table)
-  (sync-fks/sync-fks-for-table! table))
+  (let [database (table/database table)]
+    (sync-fields/sync-fields-for-table! database table)
+    (sync-fks/sync-fks-for-table! database table)
+    (sync-indexes/maybe-sync-indexes-for-table! database table)))

src/metabase/sync/sync_metadata/fields.clj

@@ -70,7 +70,7 @@
 
   ([database :- i/DatabaseInstance
     table    :- i/TableInstance]
-   (sync-util/with-error-handling (format "Errmr syncing Fields for Table ''%s''" (sync-util/name-for-logging table))
+   (sync-util/with-error-handling (format "Error syncing Fields for Table ''%s''" (sync-util/name-for-logging table))
      (let [db-metadata (fetch-metadata/db-metadata database table)]
        {:total-fields   (count db-metadata)
         :updated-fields (sync-and-update! table db-metadata)}))))

src/metabase/sync/sync_metadata/indexes.clj

@@ -0,0 +1,48 @@
+(ns metabase.sync.sync-metadata.indexes
+  (:require
+   [clojure.data :as data]
+   [metabase.driver :as driver]
+   [metabase.driver.util :as driver.u]
+   [metabase.sync.fetch-metadata :as fetch-metadata]
+   [metabase.sync.util :as sync-util]
+   [metabase.util.log :as log]
+   [toucan2.core :as t2]))
+
+(def ^:private empty-stats
+  {:total-indexes 0
+   :added-indexes 0
+   :removed-indexes 0})
+
+(defn maybe-sync-indexes-for-table!
+  "Sync the indexes for `table` if the driver supports storing index info."
+  [database table]
+  (if (driver/database-supports? (driver.u/database->driver database) :index-info database)
+    (sync-util/with-error-handling (format "Error syncing Indexes for %s" (sync-util/name-for-logging table))
+      (let [indexes                    (fetch-metadata/index-metadata database table)
+            ;; not all indexes are field names, they could be function based index as well
+            field-name-indexes         (when (seq indexes)
+                                         (t2/select-fn-set :name :model/Field :table_id (:id table) :name [:in indexes]))
+            existing-index-field-names (t2/select-fn-set :name :model/Field :table_id (:id table) :database_indexed true)
+            [removing adding]          (data/diff existing-index-field-names field-name-indexes)]
+        (doseq [field-name removing]
+          (log/infof "Unmarking %s.%s as indexed" (:name table) field-name))
+        (doseq [field-name adding]
+          (log/infof "Marking %s.%s as indexed" (:name table) field-name))
+        (if (or (seq adding) (seq removing))
+          (do (t2/update! :model/Field {:table_id (:id table)}
+                          {:database_indexed (if (seq field-name-indexes)
+                                               [:case [:in :name field-name-indexes] true :else false]
+                                               false)})
+              {:total-indexes   (count field-name-indexes)
+               :added-indexes   (count adding)
+               :removed-indexes (count removing)})
+          empty-stats)))
+    empty-stats))
+
+(defn maybe-sync-indexes!
+  "Sync the indexes for all tables in `database` if the driver supports storing index info."
+  [database]
+  (if (driver/database-supports? (driver.u/database->driver database) :index-info database)
+    (apply merge-with + empty-stats
+           (map #(maybe-sync-indexes-for-table! database %) (sync-util/db->sync-tables database)))
+    empty-stats))

test/metabase/api/database_test.clj

@@ -584,6 +584,7 @@
                                                              :has_field_values  "none"
                                                              :database_position 0
                                                              :database_required false
+                                                             :database_indexed  true
                                                              :database_is_auto_increment true})
                                                            (merge
                                                             (field-details (t2/select-one Field :id (mt/id :categories :name)))
@@ -598,6 +599,7 @@
                                                              :has_field_values  "list"
                                                              :database_position 1
                                                              :database_required true
+                                                             :database_indexed  false
                                                              :database_is_auto_increment false})]
                                      :segments     []
                                      :metrics      []

test/metabase/api/field_test.clj

@@ -69,6 +69,7 @@
                  :effective_type   "type/Text"
                  :has_field_values "list"
                  :database_required false
+                 :database_indexed  false
                  :database_is_auto_increment false
                  :dimensions       []
                  :name_field       nil})