Skip to content
Snippets Groups Projects
Unverified Commit a03dd095 authored by Ngoc Khuat's avatar Ngoc Khuat Committed by GitHub
Browse files

Detect N+1 hydration and throw an exception (#40272)

parent b277fcc7
Branches
Tags
No related merge requests found
......@@ -8,6 +8,7 @@
[clojure.walk :as walk]
[malli.core :as mc]
[malli.error :as me]
[metabase.config :as config]
[metabase.legacy-mbql.normalize :as mbql.normalize]
[metabase.legacy-mbql.schema :as mbql.s]
[metabase.lib.core :as lib]
......@@ -704,6 +705,27 @@
[_original-model dest-key _hydrated-key]
[(u/->snake_case_en (keyword (str (name dest-key) "_id")))])
(methodical/defmethod t2.hydrate/hydrate-with-strategy :around ::t2.hydrate/multimethod-simple
"Throws an error if do simple hydrations that make DB call on a sequence."
[model strategy k instances]
(if (or config/is-prod?
(< (count instances) 2)
;; we skip checking these keys because most of the times its call count
;; are from deferencing metabase.api.common/*current-user-permissions-set*
(#{:can_write :can_read} k))
(next-method model strategy k instances)
(t2/with-call-count [call-count]
(let [res (next-method model strategy k instances)
;; if it's a lazy-seq then we need to realize it so call-count is counted
res (if (instance? clojure.lang.LazySeq res)
(doall res)
res)]
;; only throws an exception if the simple hydration makes a DB call
(when (pos-int? (call-count))
(throw (ex-info (format "N+1 hydration detected!!! Model %s, key %s]" (pr-str model) k)
{:model model :strategy strategy :k k :items-count (count instances) :db-calls (call-count)})))
res))))
(mu/defn instances-with-hydrated-data
"Helper function to write batched hydrations.
Assoc to each `instances` a key `hydration-key` with data from calling `instance-key->hydrated-data-fn` by `instance-key`.
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment