Reorganize search namespaces to isolate legacy code (#48108)

dev/src/dev/search.clj

@@ -5,8 +5,6 @@
    [metabase.search.postgres.core :as search.postgres]
    [metabase.search.postgres.index :as search.index]
    [metabase.search.postgres.index-test :refer [legacy-results]]
-   [metabase.server.middleware.offset-paging :as mw.offset-paging]
-   [metabase.test :as mt]
    [toucan2.core :as t2]))
 
 (defn- basic-view [xs]
@@ -37,12 +35,13 @@
 
   (defn- mini-bench [n engine search-term & args]
     #_{:clj-kondo/ignore [:discouraged-var]}
-    (let [f (case engine
-              :index-only   search.index/search
-              :legacy       legacy-results
-              :hybrid       @#'search.postgres/hybrid
-              :hybrid-multi @#'search.postgres/hybrid-multi
-              :minimal      @#'search.postgres/minimal)]
+    (let [f (case (keyword "search.engine" (name engine))
+              :search.engine/index-only        search.index/search
+              :search.engine/legacy            legacy-results
+              :search.engine/hybrid            @#'search.postgres/hybrid
+              :search.engine/hybrid-multi      @#'search.postgres/hybrid-multi
+              :search.engine/minimal           @#'search.postgres/minimal
+              :search.engine/minimal-wth-perms @#'search.postgres/minimal-with-perms)]
       (time
        (dotimes [_ n]
          (doall (apply f search-term args))))))
@@ -51,7 +50,7 @@
   (mini-bench 500 :legacy "sample")
   ;; 30x speed-up for test-data on my machine
   (mini-bench 500 :index-only "sample")
-  ;; No noticeaable degradation, without permissions and filters
+  ;; No noticeable degradation, without permissions and filters
   (mini-bench 500 :minimal "sample")
 
   ;; but joining to the "hydrated query" reverses the advantage
@@ -61,14 +60,14 @@
   (mini-bench 100 :hybrid "sample")
   ;; using index + LIKE on the join ... still a little bit more overhead
   (mini-bench 100 :hybrid "sample" {:search-string "sample"})
-  ;; oh! this monstrocity is actually 2x faster than baseline B-)
+  ;; oh! this monstrosity is actually 2x faster than baseline B-)
   (mini-bench 100 :hybrid-multi "sample")
   (mini-bench 100 :minimal "sample"))
 
-(defn- test-search [search-string & [search-engine]]
-  (let [user-id    (mt/user->id :crowberto)
+(defn- test-search [user search-string & [search-engine]]
+  (let [user-id    (:id user)
         user-perms #{"/"}]
-    (binding [api/*current-user*                 (atom (t2/select-one :model/User user-id))
+    (binding [api/*current-user*                 (atom user)
               api/*current-user-id*              user-id
               api/*is-superuser?*                true
               api/*current-user-permissions-set* (atom user-perms)]
@@ -77,17 +76,17 @@
         {:archived                            nil
          :created-at                          nil
          :created-by                          #{}
-         :current-user-id                     379
+         :current-user-id                     user-id
          :is-superuser?                       true
          :current-user-perms                  user-perms
          :filter-items-in-personal-collection nil
          :last-edited-at                      nil
          :last-edited-by                      #{}
-         :limit                               mw.offset-paging/*limit*
+         :limit                               50
          :model-ancestors?                    nil
          :models                              search/all-models
-         :offset                              mw.offset-paging/*offset*
-         :search-engine                       search-engine
+         :offset                              0
+         :search-engine                       (some-> search-engine name)
          :search-native-query                 nil
          :search-string                       search-string
          :table-db-id                         nil
@@ -96,14 +95,21 @@
 
 (comment
   (require '[clj-async-profiler.core :as prof])
-  (prof/serve-ui 8080)
+  (prof/serve-ui 8081)
 
-  (prof/profile
-   (count
-    (dotimes [_ 100]
-      (test-search "trivia"))))
+  (let [user (t2/select-one :model/User :is_superuser true)]
+    (prof/profile
+     #_{:clj-kondo/ignore [:discouraged-var]}
+     (time
+      (count
+       (dotimes [_ 1000]
+         (test-search user "trivia"))))))
 
-  (prof/profile
-   (count
-    (dotimes [_ 1000]
-      (test-search "trivia" "minimal")))))
+  (let [user (t2/select-one :model/User :is_superuser true)]
+    (prof/profile
+     #_{:event :alloc}
+     #_{:clj-kondo/ignore [:discouraged-var]}
+     (time
+      (count
+       (dotimes [_ 1000]
+         (test-search user "trivia" :minimal)))))))

src/metabase/api/search.clj

@@ -68,7 +68,7 @@
    search_engine       [:maybe string?]
    search_native_query [:maybe true?]
    verified            [:maybe true?]}
-  (search/query-model-set
+  (search/model-set
    (search/search-context {:archived                            archived
                            :created-at                          created_at
                            :created-by                          (set (u/one-or-many created_by))

src/metabase/search.clj

 (ns metabase.search
-  "API namespace for the `metabase.search` module.
-
-  TODO: a lot of this stuff wouldn't need to be exposed if we moved more of the search stuff
-  from [[metabase.api.search]] into the `metabase.search` module."
+  "API namespace for the `metabase.search` module"
   (:require
-   [metabase.db]
+   [metabase.db :as mdb]
+   [metabase.search.api :as search.api]
    [metabase.search.config :as search.config]
+   [metabase.search.fulltext :as search.fulltext]
    [metabase.search.impl :as search.impl]
    [metabase.search.postgres.core :as search.postgres]
-   [metabase.search.scoring :as scoring]
-   [metabase.util.log :as log]
-   [metabase.util.malli :as mu]
    [potemkin :as p]))
 
 (set! *warn-on-reflection* true)
 
+(comment
+  search.api/keep-me
+  search.config/keep-me
+  search.impl/keep-me)
+
 (p/import-vars
  [search.config
   SearchableModel
   all-models]
+ [search.api
+  model-set]
  [search.impl
-  query-model-set
+  search
+  ;; We could avoid exposing this by wrapping `query-model-set` and `search` with it.
   search-context])
 
-(defn is-postgres?
-  "Check whether we can create this index"
-  []
-  (= :postgres (metabase.db/db-type)))
-
-(def ^:private default-engine :in-place)
-
-(defn- query-fn [search-engine]
-  (or
-   (case search-engine
-     :fulltext (when (is-postgres?) search.postgres/search)
-     :minimal  (when (is-postgres?) search.postgres/search)
-     :in-place search.impl/in-place
-     nil)
-
-   (log/warnf "%s search not supported for your AppDb, using %s" search-engine default-engine)
-   (recur default-engine)))
-
-(defn- model-set-fn [search-engine]
-  (or
-   (case search-engine
-     :fulltext (when (is-postgres?) search.postgres/model-set)
-     :minimal  (when (is-postgres?) search.postgres/model-set)
-     :in-place search.impl/query-model-set
-     nil)
-
-   (log/warnf "%s search not supported for your AppDb, using %s" search-engine default-engine)
-   (recur default-engine)))
-
-(defn- score-fn [search-engine]
-  (or
-   (case search-engine
-     :fulltext (when (is-postgres?) search.postgres/no-scoring)
-     :minimal  (when (is-postgres?) search.postgres/no-scoring)
-     :in-place scoring/score-and-result
-     nil)
-
-   (log/warnf "%s search not supported for your AppDb, using %s" search-engine default-engine)
-   (recur default-engine)))
+;; TODO The following need to be cleaned up to use multimethods.
 
 (defn supports-index?
-  "Does this instance support a search index, e.g. has the right kind of AppDb"
+  "Does this instance support a search index?"
   []
-  (is-postgres?))
+  (search.fulltext/supported-db? (mdb/db-type)))
 
 (defn init-index!
   "Ensure there is an index ready to be populated."
   [& {:keys [force-reset?]}]
-  (when (is-postgres?)
+  (when (supports-index?)
     (search.postgres/init! force-reset?)))
 
 (defn reindex!
   "Populate a new index, and make it active. Simultaneously updates the current index."
   []
-  (when (is-postgres?)
+  (when (supports-index?)
     (search.postgres/reindex!)))
-
-(mu/defn search
-  "Builds a search query that includes all the searchable entities and runs it"
-  [search-ctx :- search.config/SearchContext]
-  (let [engine    (:search-engine search-ctx :in-place)
-        query-fn  (query-fn engine)
-        score-fn  (score-fn engine)
-        models-fn (model-set-fn engine)]
-    (search.impl/search
-     query-fn
-     models-fn
-     score-fn
-     search-ctx)))

src/metabase/search/api.clj

+(ns metabase.search.api)
+
+;; TODO wrap these functions in Malli signatures
+
+(defmulti results "Find results matching the given search query." :search-engine)
+
+(defmulti model-set "Determine which models would have at least one result." :search-engine)
+
+(defmulti score "Rank the search results." (fn [_ {se :search-engine}] se))

src/metabase/search/config.clj

@@ -20,12 +20,6 @@
   :export?    true
   :audit      :getter)
 
-(def search-engines
-  "Supported search engines."
-  #{:in-place
-    :fulltext
-    :minimal})
-
 (def ^:dynamic *db-max-results*
   "Number of raw results to fetch from the database. This number is in place to prevent massive application DB load by
   returning tons of results; this number should probably be adjusted downward once we have UI in place to indicate
@@ -116,6 +110,8 @@
    [:current-user-perms [:set perms.u/PathSchema]]
    [:model-ancestors?   :boolean]
    [:models             [:set SearchableModel]]
+   ;; TODO this is optional only for tests, clean those up!
+   [:search-engine       {:optional true} keyword?]
    [:search-string      [:maybe ms/NonBlankString]]
    ;;
    ;; optional
@@ -127,7 +123,6 @@
    [:last-edited-by                      {:optional true} [:set {:min 1} ms/PositiveInt]]
    [:limit-int                           {:optional true} ms/Int]
    [:offset-int                          {:optional true} ms/Int]
-   [:search-engine                       {:optional true} (into [:enum] search-engines)]
    [:search-native-query                 {:optional true} true?]
    [:table-db-id                         {:optional true} ms/PositiveInt]
    ;; true to search for verified items only, nil will return all items

src/metabase/search/fulltext.clj

+(ns metabase.search.fulltext
+  (:require
+   [metabase.public-settings :as public-settings]
+   [metabase.search.api :as search.api]
+   [metabase.search.postgres.core :as search.postgres]))
+
+;; We have a bunch of experimental flavors! 🧄🌶🍊
+(derive :search.engine/hybrid :search.engine/fulltext)
+(derive :search.engine/hybrid-multi :search.engine/fulltext)
+(derive :search.engine/minimal :search.engine/fulltext)
+(derive :search.engine/minimal-with-perms :search.engine/fulltext)
+
+(defmulti supported-db? "Does the app db support fulltext search?" identity)
+
+(defmethod supported-db? :default [_] false)
+
+(defmethod supported-db? :postgres [_]
+  (public-settings/experimental-fulltext-search-enabled))
+
+;; For now we now that the app db is postgres. We can make these multimethods when that changes.
+
+(defmethod search.api/results :search.engine/fulltext
+  [search-ctx]
+  (search.postgres/search search-ctx))
+
+(defmethod search.api/model-set :search.engine/fulltext
+  [search-ctx]
+  (search.postgres/model-set search-ctx))
+
+(defmethod search.api/score :search.engine/fulltext
+  [results search-ctx]
+  (search.postgres/no-scoring results search-ctx))

src/metabase/search/legacy.clj

+(ns metabase.search.legacy
+  (:require
+   [honey.sql.helpers :as sql.helpers]
+   [medley.core :as m]
+   [metabase.db :as mdb]
+   [metabase.db.query :as mdb.query]
+   [metabase.models.collection :as collection]
+   [metabase.models.permissions :as perms]
+   [metabase.search.api :as search.api]
+   [metabase.search.config
+    :as search.config
+    :refer [SearchContext SearchableModel]]
+   [metabase.search.filter :as search.filter]
+   [metabase.search.scoring :as scoring]
+   [metabase.search.util :as search.util]
+   [metabase.util :as u]
+   [metabase.util.honey-sql-2 :as h2x]
+   [metabase.util.log :as log]
+   [metabase.util.malli :as mu]
+   [metabase.util.malli.schema :as ms]
+   [toucan2.core :as t2]))
+
+(def ^:private HoneySQLColumn
+  [:or
+   :keyword
+   [:tuple :any :keyword]])
+
+(mu/defn- ->column-alias :- keyword?
+  "Returns the column name. If the column is aliased, i.e. [`:original_name` `:aliased_name`], return the aliased
+  column name"
+  [column-or-aliased :- HoneySQLColumn]
+  (if (sequential? column-or-aliased)
+    (second column-or-aliased)
+    column-or-aliased))
+
+(mu/defn- canonical-columns :- [:sequential HoneySQLColumn]
+  "Returns a seq of lists of canonical columns for the search query with the given `model` Will return column names
+  prefixed with the `model` name so that it can be used in criteria. Projects a `nil` for columns the `model` doesn't
+  have and doesn't modify aliases."
+  [model :- SearchableModel, col-alias->honeysql-clause :- [:map-of :keyword HoneySQLColumn]]
+  (for [[search-col col-type] search.config/all-search-columns
+        :let [maybe-aliased-col (get col-alias->honeysql-clause search-col)]]
+    (cond
+      (= search-col :model)
+      [(h2x/literal model) :model]
+
+      ;; This is an aliased column, no need to include the table alias
+      (sequential? maybe-aliased-col)
+      maybe-aliased-col
+
+      ;; This is a column reference, need to add the table alias to the column
+      maybe-aliased-col
+      (search.config/column-with-model-alias model maybe-aliased-col)
+
+      ;; This entity is missing the column, project a null for that column value. For Postgres and H2, cast it to the
+      ;; correct type, e.g.,
+      ;;
+      ;;    SELECT cast(NULL AS integer)
+      ;;
+      ;; For MySQL, this is not needed.
+      :else
+      [(when-not (= (mdb/db-type) :mysql)
+         [:cast nil col-type])
+       search-col])))
+
+(mu/defn- add-table-db-id-clause
+  "Add a WHERE clause to only return tables with the given DB id.
+  Used in data picker for joins because we can't join across DB's."
+  [query :- ms/Map id :- [:maybe ms/PositiveInt]]
+  (if (some? id)
+    (sql.helpers/where query [:= id :db_id])
+    query))
+
+(mu/defn- add-card-db-id-clause
+  "Add a WHERE clause to only return cards with the given DB id.
+  Used in data picker for joins because we can't join across DB's."
+  [query :- ms/Map id :- [:maybe ms/PositiveInt]]
+  (if (some? id)
+    (sql.helpers/where query [:= id :database_id])
+    query))
+
+(mu/defn- replace-select :- :map
+  "Replace a select from query that has alias is `target-alias` with [`with` `target-alias`] column, throw an error if
+  can't find the target select.
+
+  This works with the assumption that `query` contains a list of select from [[select-clause-for-model]],
+  and some of them are dummy column casted to the correct type.
+
+  This function then will replace the dummy column with alias is `target-alias` with the `with` column."
+  [query :- :map
+   target-alias :- :keyword
+   with :- :keyword]
+  (let [selects     (:select query)
+        idx         (first (keep-indexed (fn [index item]
+                                           (when (and (coll? item)
+                                                      (= (last item) target-alias))
+                                             index))
+                                         selects))
+        with-select [with target-alias]]
+    (if (some? idx)
+      (assoc query :select (m/replace-nth idx with-select selects))
+      (throw (ex-info "Failed to replace selector" {:status-code  400
+                                                    :target-alias target-alias
+                                                    :with         with})))))
+
+(mu/defn- with-last-editing-info :- :map
+  [query :- :map
+   model :- [:enum "card" "dashboard"]]
+  (-> query
+      (replace-select :last_editor_id :r.user_id)
+      (replace-select :last_edited_at :r.timestamp)
+      (sql.helpers/left-join [:revision :r]
+                             [:and [:= :r.model_id (search.config/column-with-model-alias model :id)]
+                              [:= :r.most_recent true]
+                              [:= :r.model (search.config/search-model->revision-model model)]])))
+
+(mu/defn- with-moderated-status :- :map
+  [query :- :map
+   model :- [:enum "card" "dataset"]]
+  (-> query
+      (replace-select :moderated_status :mr.status)
+      (sql.helpers/left-join [:moderation_review :mr]
+                             [:and
+                              [:= :mr.moderated_item_type "card"]
+                              [:= :mr.moderated_item_id (search.config/column-with-model-alias model :id)]
+                              [:= :mr.most_recent true]])))
+
+(defn order-clause
+  "CASE expression that lets the results be ordered by whether they're an exact (non-fuzzy) match or not"
+  [query]
+  (let [match             (search.util/wildcard-match (search.util/normalize query))
+        columns-to-search (->> search.config/all-search-columns
+                               (filter (fn [[_k v]] (= v :text)))
+                               (map first)
+                               (remove #{:collection_authority_level :moderated_status
+                                         :initial_sync_status :pk_ref :location
+                                         :collection_location}))
+        case-clauses      (as-> columns-to-search <>
+                            (map (fn [col] [:like [:lower col] match]) <>)
+                            (interleave <> (repeat [:inline 0]))
+                            (concat <> [:else [:inline 1]]))]
+    [(into [:case] case-clauses)]))
+
+(defmulti search-query-for-model
+  "Build a HoneySQL query with all the data relevant to a given model, padded
+  with NULL fields to support UNION queries."
+  {:arglists '([model search-context])}
+  (fn [model _] model))
+
+(mu/defn- select-clause-for-model :- [:sequential HoneySQLColumn]
+  "The search query uses a `union-all` which requires that there be the same number of columns in each of the segments
+  of the query. This function will take the columns for `model` and will inject constant `nil` values for any column
+  missing from `entity-columns` but found in `search.config/all-search-columns`."
+  [model :- SearchableModel]
+  (let [entity-columns                (search.config/columns-for-model model)
+        column-alias->honeysql-clause (m/index-by ->column-alias entity-columns)
+        cols-or-nils                  (canonical-columns model column-alias->honeysql-clause)]
+    cols-or-nils))
+
+(mu/defn- from-clause-for-model :- [:tuple [:tuple :keyword :keyword]]
+  [model :- SearchableModel]
+  (let [{:keys [db-model alias]} (get search.config/model-to-db-model model)]
+    [[(t2/table-name db-model) alias]]))
+
+(mu/defn- base-query-for-model :- [:map {:closed true}
+                                   [:select :any]
+                                   [:from :any]
+                                   [:where {:optional true} :any]
+                                   [:join {:optional true} :any]
+                                   [:left-join {:optional true} :any]]
+  "Create a HoneySQL query map with `:select`, `:from`, and `:where` clauses for `model`, suitable for the `UNION ALL`
+  used in search."
+  [model :- SearchableModel context :- SearchContext]
+  (-> {:select (select-clause-for-model model)
+       :from   (from-clause-for-model model)}
+      (search.filter/build-filters model context)))
+
+(mu/defn add-collection-join-and-where-clauses
+  "Add a `WHERE` clause to the query to only return Collections the Current User has access to; join against Collection,
+  so we can return its `:name`."
+  [honeysql-query :- ms/Map
+   model :- :string
+   {:keys [filter-items-in-personal-collection
+           archived
+           current-user-id
+           is-superuser?]} :- SearchContext]
+  (let [collection-id-col        (if (= model "collection")
+                                   :collection.id
+                                   :collection_id)
+        collection-filter-clause (collection/visible-collection-filter-clause
+                                  collection-id-col
+                                  {:include-archived-items    :all
+                                   :include-trash-collection? true
+                                   :permission-level          (if archived
+                                                                :write
+                                                                :read)}
+                                  {:current-user-id current-user-id
+                                   :is-superuser?   is-superuser?})]
+    (cond-> honeysql-query
+      true
+      (sql.helpers/where collection-filter-clause (perms/audit-namespace-clause :collection.namespace nil))
+      ;; add a JOIN against Collection *unless* the source table is already Collection
+      (not= model "collection")
+      (sql.helpers/left-join [:collection :collection]
+                             [:= collection-id-col :collection.id])
+
+      (some? filter-items-in-personal-collection)
+      (sql.helpers/where
+       (case filter-items-in-personal-collection
+         "only"
+         (concat [:or]
+                 ;; sub personal collections
+                 (for [id (t2/select-pks-set :model/Collection :personal_owner_id [:not= nil])]
+                   [:like :collection.location (format "/%d/%%" id)])
+                 ;; top level personal collections
+                 [[:and
+                   [:= :collection.location "/"]
+                   [:not= :collection.personal_owner_id nil]]])
+
+         "exclude"
+         (conj [:or]
+               (into
+                [:and [:= :collection.personal_owner_id nil]]
+                (for [id (t2/select-pks-set :model/Collection :personal_owner_id [:not= nil])]
+                  [:not-like :collection.location (format "/%d/%%" id)]))
+               [:= collection-id-col nil]))))))
+
+(mu/defn- shared-card-impl
+  [model :- :metabase.models.card/type
+   search-ctx :- SearchContext]
+  (-> (base-query-for-model "card" search-ctx)
+      (sql.helpers/where [:= :card.type (name model)])
+      (sql.helpers/left-join [:card_bookmark :bookmark]
+                             [:and
+                              [:= :bookmark.card_id :card.id]
+                              [:= :bookmark.user_id (:current-user-id search-ctx)]])
+      (add-collection-join-and-where-clauses "card" search-ctx)
+      (add-card-db-id-clause (:table-db-id search-ctx))
+      (with-last-editing-info "card")
+      (with-moderated-status "card")))
+
+(defmethod search-query-for-model "action"
+  [model search-ctx]
+  (-> (base-query-for-model model search-ctx)
+      (sql.helpers/left-join [:report_card :model]
+                             [:= :model.id :action.model_id])
+      (sql.helpers/left-join :query_action
+                             [:= :query_action.action_id :action.id])
+      (add-collection-join-and-where-clauses model search-ctx)))
+
+(defmethod search-query-for-model "card"
+  [_model search-ctx]
+  (shared-card-impl :question search-ctx))
+
+(defmethod search-query-for-model "dataset"
+  [_model search-ctx]
+  (-> (shared-card-impl :model search-ctx)
+      (update :select (fn [columns]
+                        (cons [(h2x/literal "dataset") :model] (rest columns))))))
+
+(defmethod search-query-for-model "metric"
+  [_model search-ctx]
+  (-> (shared-card-impl :metric search-ctx)
+      (update :select (fn [columns]
+                        (cons [(h2x/literal "metric") :model] (rest columns))))))
+
+(defmethod search-query-for-model "collection"
+  [model search-ctx]
+  (-> (base-query-for-model "collection" search-ctx)
+      (sql.helpers/left-join [:collection_bookmark :bookmark]
+                             [:and
+                              [:= :bookmark.collection_id :collection.id]
+                              [:= :bookmark.user_id (:current-user-id search-ctx)]])
+      (add-collection-join-and-where-clauses model search-ctx)))
+
+(defmethod search-query-for-model "database"
+  [model search-ctx]
+  (base-query-for-model model search-ctx))
+
+(defmethod search-query-for-model "dashboard"
+  [model search-ctx]
+  (-> (base-query-for-model model search-ctx)
+      (sql.helpers/left-join [:dashboard_bookmark :bookmark]
+                             [:and
+                              [:= :bookmark.dashboard_id :dashboard.id]
+                              [:= :bookmark.user_id (:current-user-id search-ctx)]])
+      (add-collection-join-and-where-clauses model search-ctx)
+      (with-last-editing-info "dashboard")))
+
+(defn- add-model-index-permissions-clause
+  [query {:keys [current-user-id is-superuser?]}]
+  (sql.helpers/where
+   query
+   (collection/visible-collection-filter-clause
+    :collection_id
+    {}
+    {:current-user-id current-user-id
+     :is-superuser?   is-superuser?})))
+
+(defmethod search-query-for-model "indexed-entity"
+  [model search-ctx]
+  (-> (base-query-for-model model search-ctx)
+      (sql.helpers/left-join [:model_index :model-index]
+                             [:= :model-index.id :model-index-value.model_index_id])
+      (sql.helpers/left-join [:report_card :model] [:= :model-index.model_id :model.id])
+      (sql.helpers/left-join [:collection :collection] [:= :model.collection_id :collection.id])
+      (add-model-index-permissions-clause search-ctx)))
+
+(defmethod search-query-for-model "segment"
+  [model search-ctx]
+  (-> (base-query-for-model model search-ctx)
+      (sql.helpers/left-join [:metabase_table :table] [:= :segment.table_id :table.id])))
+
+(defmethod search-query-for-model "table"
+  [model {:keys [current-user-perms table-db-id], :as search-ctx}]
+  (when (seq current-user-perms)
+
+    (-> (base-query-for-model model search-ctx)
+        (add-table-db-id-clause table-db-id)
+        (sql.helpers/left-join :metabase_database [:= :table.db_id :metabase_database.id]))))
+
+(defmethod search.api/model-set :search.engine/in-place
+  [search-ctx]
+  (let [model-queries (for [model (search.filter/search-context->applicable-models
+                                   ;; It's unclear why we don't use the existing :models
+                                   (assoc search-ctx :models search.config/all-models))]
+                        {:nest (sql.helpers/limit (search-query-for-model model search-ctx) 1)})
+        query         (when (pos-int? (count model-queries))
+                        {:select [:*]
+                         :from   [[{:union-all model-queries} :dummy_alias]]})]
+    (into #{} (map :model) (some-> query mdb.query/query))))
+
+(mu/defn full-search-query
+  "Postgres 9 is not happy with the type munging it needs to do to make the union-all degenerate down to a trivial case
+  of one model without errors. Therefore, we degenerate it down for it"
+  [search-ctx :- SearchContext]
+  (let [models       (:models search-ctx)
+        order-clause [((fnil order-clause "") (:search-string search-ctx))]]
+    (cond
+      (= (count models) 0)
+      {:select [nil]}
+
+      (= (count models) 1)
+      (merge (search-query-for-model (first models) search-ctx)
+             {:limit search.config/*db-max-results*})
+
+      :else
+      {:select   [:*]
+       :from     [[{:union-all (vec (for [model models
+                                          :let [query (search-query-for-model model search-ctx)]
+                                          :when (seq query)]
+                                      query))} :alias_is_required_by_sql_but_not_needed_here]]
+       :order-by order-clause
+       :limit    search.config/*db-max-results*})))
+
+;; Return a reducible-query corresponding to searching the entities without an index.
+(defmethod search.api/results
+  :search.engine/in-place
+  [search-ctx]
+  (let [search-query (full-search-query search-ctx)]
+    (log/tracef "Searching with query:\n%s\n%s"
+                (u/pprint-to-str search-query)
+                (mdb.query/format-sql (first (mdb.query/compile search-query))))
+    (t2/reducible-query search-query)))
+
+(defmethod search.api/score :search.engine/in-place [results search-ctx]
+  (scoring/score-and-result results search-ctx))

src/metabase/search/postgres/core.clj

@@ -5,7 +5,7 @@
    [honey.sql.helpers :as sql.helpers]
    [metabase.api.common :as api]
    [metabase.search.config :as search.config]
-   [metabase.search.impl :as search.impl]
+   [metabase.search.legacy :as search.legacy]
    [metabase.search.postgres.index :as search.index]
    [metabase.search.postgres.ingestion :as search.ingestion]
    [toucan2.core :as t2])
@@ -31,25 +31,25 @@
      :current-user-perms #{"/"}}))
 
 (defn- in-place-query [{:keys [models search-term archived?] :as search-ctx}]
-  (search.impl/full-search-query
+  (search.legacy/full-search-query
    (merge
     (user-params search-ctx)
-    {:search-string      search-term
-     :models             (or models
-                             (if api/*current-user-id*
-                               search.config/all-models
-                               ;; For REPL convenience, skip these models as
-                               ;; they require the user to be initialized.
-                               (disj search.config/all-models "indexed-entity")))
-     :archived?          archived?
-     :model-ancestors?   true})))
+    {:search-string    search-term
+     :models           (or models
+                           (if api/*current-user-id*
+                             search.config/all-models
+                             ;; For REPL convenience, skip these models as
+                             ;; they require the user to be initialized.
+                             (disj search.config/all-models "indexed-entity")))
+     :archived?        archived?
+     :model-ancestors? true})))
 
 (defn- hybrid
-  "Use the index for appling the search string, but rely on the legacy code path for rendering
+  "Use the index for using the search string, but rely on the legacy code path for rendering
   the display data, applying permissions, additional filtering, etc.
 
   NOTE: this is less efficient than legacy search even. We plan to replace it with something
-  less feature complete, but much faster."
+  less feature complete but much faster."
   [search-term & {:as search-ctx}]
   (when-not @#'search.index/initialized?
     (throw (ex-info "Search index is not initialized. Use [[init!]] to ensure it exists."
@@ -115,7 +115,7 @@
   (when-not @#'search.index/initialized?
     (throw (ex-info "Search index is not initialized. Use [[init!]] to ensure it exists."
                     {:search-engine :postgres})))
-  (->> (search.impl/add-collection-join-and-where-clauses
+  (->> (search.legacy/add-collection-join-and-where-clauses
         (assoc (search.index/search-query search-term)
                :select [:legacy_input])
         ;; we just need this to not be "collection"
@@ -133,11 +133,11 @@
 
 (defn- search-fn [search-engine]
   (case search-engine
-    :hybrid             hybrid
-    :hubrid-multi       hybrid-multi
-    :minimal            minimal
-    :minimal-with-perms minimal-with-perms
-    :fulltext           default-engine
+    :search.engine/hybrid             hybrid
+    :search.engine/hybrid-multi       hybrid-multi
+    :search.engine/minimal            minimal
+    :search.engine/minimal-with-perms minimal-with-perms
+    :search.engine/fulltext           default-engine
     default-engine))
 
 (defn search
@@ -162,9 +162,9 @@
     (:models search-ctx search.config/all-models))))
 
 (defn no-scoring
-  "Do no scoring, whatsover"
+  "Do no scoring, whatsoever"
   [result _scoring-ctx]
-  {:score 1
+  {:score  1
    :result (assoc result :all-scores [] :relevant-scores [])})
 
 (defn init!

src/metabase/search/postgres/index.clj

@@ -42,7 +42,7 @@
             [[:id :bigint [:primary-key] [:raw "GENERATED BY DEFAULT AS IDENTITY"]]
              ;; entity
              [:model_id :int :not-null]
-             [:model [:varchar 254] :not-null] ;; TODO find the right size
+             [:model [:varchar 254] :not-null] ;; TODO We could shrink this to just what we need.
              ;; search
              [:search_vector :tsvector :not-null]
              ;; results
@@ -60,9 +60,10 @@
              [:created_at :timestamp
               [:default [:raw "CURRENT_TIMESTAMP"]]
               :not-null]])
-
           t2/query)
 
+      ;; TODO I strongly suspect that there are more indexes that would help performance, we should examine EXPLAIN.
+
       (t2/query
        (format "CREATE INDEX IF NOT EXISTS %s_tsvector_idx ON %s USING gin (search_vector)"
                (str/replace (str (name active-table) "_" (random-uuid)) #"-" "_")
@@ -70,7 +71,7 @@
     (reset! reindexing? true)))
 
 (defn activate-pending!
-  "Make the pending index active, if it exists. Returns true if it did so."
+  "Make the pending index active if it exists. Returns true if it did so."
   []
   ;; ... just in case it wasn't cleaned up last time.
   (drop-table! retired-table)
@@ -152,6 +153,7 @@
   [input]
   (let [trimmed        (str/trim input)
         complete?      (not (str/ends-with? trimmed "\""))
+        ;; TODO also only complete if search-typeahead-enabled and the context is the search palette
         maybe-complete (if complete? complete-last-word identity)]
     (->> (split-preserving-quotes trimmed)
          (remove str/blank?)
@@ -171,7 +173,7 @@
       (t2/insert! pending-table entries))))
 
 (defn search-query
-  "Query fragment for all models corresponding to a query paramter `:search-term`."
+  "Query fragment for all models corresponding to a query parameter `:search-term`."
   [search-term]
   {:select [:model_id :model]
    :from   [active-table]
@@ -189,7 +191,7 @@
        (t2/query (search-query search-term))))
 
 (defn reset-index!
-  "Ensure we have a blank slate, in case the table schema or stored data format has changed."
+  "Ensure we have a blank slate; in case the table schema or stored data format has changed."
   []
   (reset! reindexing? false)
   (drop-table! pending-table)

src/metabase/search/postgres/ingestion.clj

@@ -6,7 +6,7 @@
   (:require
    [clojure.string :as str]
    [metabase.search.config :as search.config]
-   [metabase.search.impl :as search.impl]
+   [metabase.search.legacy :as search.legacy]
    [metabase.search.postgres.index :as search.index]
    [toucan2.core :as t2]
    [toucan2.realize :as t2.realize]))
@@ -57,7 +57,7 @@
        :archived?          nil
        ;; only need this for display data
        :model-ancestors?   false}
-      search.impl/full-search-query
+      search.legacy/full-search-query
       (dissoc :limit)
       t2/reducible-query))
 

test/metabase/search/impl_test.clj

@@ -10,6 +10,7 @@
    [metabase.config :as config]
    [metabase.search.config :as search.config]
    [metabase.search.impl :as search.impl]
+   [metabase.search.legacy :as search.legacy]
    [metabase.test :as mt]
    [toucan2.core :as t2]
    [toucan2.tools.with-temp :as t2.with-temp]))
@@ -31,7 +32,7 @@
              [:like [:lower :model_name]        "%foo%"] [:inline 0]
              [:like [:lower :dataset_query]     "%foo%"] [:inline 0]
              :else [:inline 1]]]
-           (search.impl/order-clause "Foo")))))
+           (search.legacy/order-clause "Foo")))))
 
 (deftest search-db-call-count-test
   (let [search-string (mt/random-name)]

test/metabase/search/postgres/core_test.clj

@@ -2,7 +2,8 @@
   (:require
    [clojure.string :as str]
    [clojure.test :refer [deftest is testing]]
-   [metabase.search :as search :refer [is-postgres?]]
+   [metabase.db :as mdb]
+   [metabase.search :as search]
    [metabase.search.postgres.core :as search.postgres]
    [metabase.search.postgres.index-test :refer [legacy-results]]
    [metabase.test :as mt]
@@ -13,7 +14,7 @@
 
 #_{:clj-kondo/ignore [:metabase/test-helpers-use-non-thread-safe-functions]}
 (defmacro with-setup [& body]
-  `(when (is-postgres?)
+  `(when (= :postgres (mdb/db-type))
      ;; TODO add more extensive data to search
      (mt/dataset ~'test-data
        (search.postgres/init! true)

test/metabase/search/postgres/index_test.clj

 (ns metabase.search.postgres.index-test
   (:require
    [clojure.test :refer [deftest is testing]]
-   [metabase.search :refer [is-postgres?]]
+   [metabase.db :as mdb]
    [metabase.search.postgres.core :as search.postgres]
    [metabase.search.postgres.index :as search.index]
    [metabase.search.postgres.ingestion :as search.ingestion]
@@ -11,7 +11,7 @@
 (defn legacy-results
   "Use the source tables directly to search for records."
   [search-term & {:as opts}]
-  (t2/query (#'search.postgres/in-place-query (assoc opts :search-term search-term))))
+  (t2/query (#'search.postgres/in-place-query (assoc opts :search-engine :search.engine/in-place :search-term search-term))))
 
 (def legacy-models
   "Just the identity of the matches"
@@ -27,7 +27,7 @@
 (defmacro with-index
   "Ensure a clean, small index."
   [& body]
-  `(when (is-postgres?)
+  `(when (= :postgres (mdb/db-type))
      (mt/dataset ~(symbol "test-data")
        (mt/with-temp [:model/Card {} {:name "Customer Satisfaction" :collection_id 1}
                       :model/Card {} {:name "The Latest Revenue Projections" :collection_id 1}

test/metabase/search/scoring_test.clj

@@ -4,7 +4,7 @@
    [java-time.api :as t]
    [metabase.search.config :as search.config]
    [metabase.search.filter-test :as search.filter-test]
-   [metabase.search.impl :as search.impl]
+   [metabase.search.legacy :as search.legacy]
    [metabase.search.scoring :as scoring]
    [metabase.test :as mt]
    [toucan2.core :as t2]))
@@ -235,7 +235,7 @@
   [search-ctx]
   (mt/with-current-user (mt/user->id :crowberto)
     (let [search-ctx (merge search.filter-test/default-search-ctx search-ctx)]
-      (t2/query (#'search.impl/full-search-query search-ctx)))))
+      (t2/query (search.legacy/full-search-query search-ctx)))))
 
 (deftest search-native-query-scoring-test
   (testing "Exclude native query matches in search scoring when the search should exclude native queries"