Optimise autocomplete query (#38080)

For big databases autocomplete can be quite slow. At stats, query in regular form takes about 80 ms to run. Filtering out inactive tables and adding an index (pgsql-only unfortunately) for ordering clause brings that down to 10 ms.

Also I suspect that in a single session you'll hit similar autocomplete rules multiple time, so some client caching could help with performance.

resources/migrations/001_update_migrations.yaml

@@ -5205,6 +5205,29 @@ databaseChangeLog:
         - dbms:
             type: mysql,mariadb
 
+  - changeSet:
+      id: v49.2023-01-24T12:00:00
+      author: piranha
+      comment: >-
+        This significantly speeds up api.database/autocomplete-fields query
+        and is an improvement for issue 30588.
+        H2 does not support this: https://github.com/h2database/h2database/issues/3535
+        Mariadb does not support this.
+        Mysql says it does, but reports an error during migration.
+      dbms: postgresql
+      changes:
+        - createIndex:
+            tableName: metabase_field
+            indexName: idx_field_name_lower
+            columns:
+              - column:
+                  name: lower(name)
+                  computed: true
+      rollback:
+        - dropIndex:
+            tableName: metabase_field
+            indexName: idx_field_name_lower
+
   # >>>>>>>>>> DO NOT ADD NEW MIGRATIONS BELOW THIS LINE! ADD THEM ABOVE <<<<<<<<<<
 
 ########################################################################################################################

src/metabase/api/database.clj

@@ -546,15 +546,21 @@
                 :limit    50})))
 
 (defn- autocomplete-fields [db-id search-string limit]
+  ;; NOTE: measuring showed that this query performance is improved ~4x when adding trgm index in pgsql and ~10x when
+  ;; adding a index on `lower(metabase_field.name)` for ordering (trgm index having on impact on queries with index).
+  ;; Pgsql now has an index on that (see migration `v49.2023-01-24T12:00:00`) as other dbms do not support indexes on
+  ;; expressions.
   (t2/select [Field :name :base_type :semantic_type :id :table_id [:table.name :table_name]]
              :metabase_field.active          true
              :%lower.metabase_field/name     [:like (u/lower-case-en search-string)]
              :metabase_field.visibility_type [:not-in ["sensitive" "retired"]]
              :table.db_id                    db-id
-             {:order-by  [[[:lower :metabase_field.name] :asc]
-                          [[:lower :table.name] :asc]]
-              :left-join [[:metabase_table :table] [:= :table.id :metabase_field.table_id]]
-              :limit     limit}))
+             {:order-by   [[[:lower :metabase_field.name] :asc]
+                           [[:lower :table.name] :asc]]
+              ;; checking for table.active in join makes query faster when there are a lot of inactive tables
+              :inner-join [[:metabase_table :table] [:and :table.active
+                                                     [:= :table.id :metabase_field.table_id]]]
+              :limit      limit}))
 
 (defn- autocomplete-results [tables fields limit]
   (let [tbl-count   (count tables)
@@ -620,11 +626,16 @@
   (when (and (str/blank? prefix) (str/blank? substring))
     (throw (ex-info (tru "Must include prefix or search") {:status-code 400})))
   (try
-    (cond
-      substring
-      (autocomplete-suggestions id (str "%" substring "%"))
-      prefix
-      (autocomplete-suggestions id (str prefix "%")))
+    {:status  200
+     ;; Presumably user will repeat same prefixes many times writing the query,
+     ;; so let them cache response to make autocomplete feel fast. 60 seconds
+     ;; is not enough to be a nuisance when schema or permissions change. Cache
+     ;; is user-specific since we're checking for permissions.
+     :headers {"Cache-Control" "public, max-age=60"
+               "Vary"          "Cookie"}
+     :body    (cond
+                substring (autocomplete-suggestions id (str "%" substring "%"))
+                prefix    (autocomplete-suggestions id (str prefix "%")))}
     (catch Throwable e
       (log/warn e (trs "Error with autocomplete: {0}" (ex-message e))))))
 

src/metabase/server/middleware/security.clj

@@ -168,10 +168,11 @@
     "X-Content-Type-Options"            "nosniff"}))
 
 (defn- add-security-headers* [request response]
-  (update response :headers merge (security-headers
-                                   :nonce          (:nonce request)
-                                   :allow-iframes? ((some-fn request.u/public? request.u/embed?) request)
-                                   :allow-cache?   (request.u/cacheable? request))))
+  ;; merge is other way around so that handler can override headers
+  (update response :headers #(merge %2 %1) (security-headers
+                                            :nonce          (:nonce request)
+                                            :allow-iframes? ((some-fn request.u/public? request.u/embed?) request)
+                                            :allow-cache?   (request.u/cacheable? request))))
 
 (defn add-security-headers
   "Middleware that adds HTTP security and cache-busting headers."

test/metabase/api/database_test.clj

@@ -11,6 +11,7 @@
    [metabase.driver.h2 :as h2]
    [metabase.driver.sql-jdbc.execute :as sql-jdbc.execute]
    [metabase.driver.util :as driver.u]
+   [metabase.http-client :as client]
    [metabase.lib.schema.id :as lib.schema.id]
    [metabase.models
     :refer [Card Collection Database Field FieldValues Metric Segment Table]]
@@ -27,6 +28,7 @@
    [metabase.test :as mt]
    [metabase.test.data.impl :as data.impl]
    [metabase.test.data.interface :as tx]
+   [metabase.test.data.users :as test.users]
    [metabase.test.fixtures :as fixtures]
    [metabase.test.util :as tu]
    [metabase.util :as u]
@@ -683,6 +685,13 @@
                                         ["CATEGORY" "PRODUCTS :type/Text :type/Category"]
                                         ["CATEGORY_ID" "VENUES :type/Integer :type/FK"]]}]
         (is (= expected (prefix-fn (mt/id) prefix))))
+      (testing " returns sane Cache-Control headers"
+        (is (=? {"Cache-Control" "public, max-age=60"
+                 "Vary"          "Cookie"}
+                (-> (client/client-full-response (test.users/username->token :rasta) :get 200
+                                                (format "database/%s/autocomplete_suggestions" (mt/id))
+                                                :prefix "u")
+                   :headers))))
       (testing " handles large numbers of tables and fields sensibly with prefix"
         (mt/with-model-cleanup [Field Table Database]
           (let [tmp-db (first (t2/insert-returning-instances! Database {:name "Temp Autocomplete Pagination DB" :engine "h2" :details "{}"}))]