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

move the detect n+1 hydration to dev ns (#40486)

parent c6cd140e
Branches
Tags
No related merge requests found
......@@ -70,6 +70,7 @@
[potemkin :as p]
[toucan2.connection :as t2.connection]
[toucan2.core :as t2]
[toucan2.hydrate :as t2.hydrate]
[toucan2.pipeline :as t2.pipeline]))
(set! *warn-on-reflection* true)
......@@ -297,6 +298,27 @@
(qp.compile/compile built-query))]
(into [query] params)))
(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))))
(defn app-db-as-data-warehouse
"Add the application database as a Database. Currently only works if your app DB uses broken-out details!"
[]
......
......@@ -8,7 +8,6 @@
[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]
......@@ -705,27 +704,6 @@
[_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