From e98b6a14b458f542a77e84506d9190995fe0a998 Mon Sep 17 00:00:00 2001
From: metamben <103100869+metamben@users.noreply.github.com>
Date: Fri, 12 Jan 2024 18:16:14 +0300
Subject: [PATCH] Optimize canonicalization of MBQL clauses (#37539)
* Optimize canonicalization of MBQL clauses
Fixes #37245.
---
src/metabase/mbql/normalize.cljc | 66 ++++++++++++++++++++++----------
1 file changed, 46 insertions(+), 20 deletions(-)
diff --git a/src/metabase/mbql/normalize.cljc b/src/metabase/mbql/normalize.cljc
index 853c584a7d0..982f5917828 100644
--- a/src/metabase/mbql/normalize.cljc
+++ b/src/metabase/mbql/normalize.cljc
@@ -639,25 +639,45 @@
(defn- canonicalize-mbql-clauses
"Walk an `mbql-query` an canonicalize non-top-level clauses like `:fk->`."
- [mbql-query]
- (walk/prewalk
- (fn [x]
- (cond
- (map? x)
- (m/map-vals canonicalize-mbql-clauses x)
-
- (not (mbql-clause? x))
- x
-
- :else
- (try
- (canonicalize-mbql-clause x)
- (catch #?(:clj Throwable :cljs js/Error) e
- (log/error (i18n/tru "Invalid clause:") x)
- (throw (ex-info (i18n/tru "Invalid MBQL clause: {0}" (ex-message e))
- {:clause x}
- e))))))
- mbql-query))
+ [form]
+ (cond
+ ;; Special handling for records so that they are not converted into plain maps.
+ ;; Only the values are canonicalized.
+ (record? form)
+ (reduce-kv (fn [r k x] (assoc r k (canonicalize-mbql-clauses x))) form form)
+
+ ;; Only the values are canonicalized.
+ (map? form)
+ (update-vals form canonicalize-mbql-clauses)
+
+ (mbql-clause? form)
+ (let [top-canonical
+ (try
+ (canonicalize-mbql-clause form)
+ (catch #?(:clj Throwable :cljs js/Error) e
+ (log/error (i18n/tru "Invalid clause:") form)
+ (throw (ex-info (i18n/tru "Invalid MBQL clause: {0}" (ex-message e))
+ {:clause form}
+ e))))]
+ ;; Canonical clauses are assumed to be sequential things conj'd at the end.
+ ;; In fact, they should better be vectors.
+ (if (seq top-canonical)
+ (into (conj (empty top-canonical) (first top-canonical))
+ (map canonicalize-mbql-clauses)
+ (rest top-canonical))
+ top-canonical))
+
+ ;; ISeq instances (e.g., list and lazy sequences) are converted to vectors.
+ (seq? form)
+ (mapv canonicalize-mbql-clauses form)
+
+ ;; Other collections (e.g., vectors, sets, and queues) are assumed to be conj'd at the end
+ ;; and we keep their types.
+ (coll? form)
+ (into (empty form) (map canonicalize-mbql-clauses) form)
+
+ :else
+ form))
(defn- wrap-single-aggregations
"Convert old MBQL 95 single-aggregations like `{:aggregation :count}` or `{:aggregation [:count]}` to MBQL 98+
@@ -764,6 +784,12 @@
(dissoc :source-metadata)
(assoc-in [:query :source-metadata] source-metadata)))
+(defn- canonicalize-mbql-clauses-excluding-native
+ [{:keys [native] :as outer-query}]
+ (if native
+ (-> outer-query (dissoc :native) canonicalize-mbql-clauses (assoc :native native))
+ (canonicalize-mbql-clauses outer-query)))
+
(defn- canonicalize
"Canonicalize a query [MBQL query], rewriting the query as if you perfectly followed the recommended style guides for
writing MBQL. Does things like removes unneeded and empty clauses, converts older MBQL '95 syntax to MBQL '98, etc."
@@ -774,7 +800,7 @@
query (update :query canonicalize-inner-mbql-query)
parameters (update :parameters (partial mapv canonicalize-mbql-clauses))
native (update :native canonicalize-native-query)
- true canonicalize-mbql-clauses)
+ true canonicalize-mbql-clauses-excluding-native)
(catch #?(:clj Throwable :cljs js/Error) e
(throw (ex-info (i18n/tru "Error canonicalizing query: {0}" (ex-message e))
{:query query}
--
GitLab