Rename metabase.mbql to metabase.legacy-mbql (metabase#40158)

* Wow * Test fix 🔧 * Fixes * Actions should use strings for column names (fix :update-row and :create-row normalization) * MLv2 schema should check against keys for the other query type * Ok, have I fixed things? * More fixes 🔧 * Fix indentation * Another round of test fixes. 🔧 * Hopefully the last few test fixes 🔧 * We need to test normalization for queries that have keyword keys as well. * Fix Cljs i18n namespaces * Sort namespaces * Only test against H2 * Rename `metabase.mbql` to `metabase.legacy-mbql` * Fix Kondo warnings * Test fixes 🔧 * Register MBQL clause schemas and test fixes 🔧 * Test fixes and PR feedback * Test fix * Remove the normalization tests * Test fixes 🔧 * Fix kondo * Fix import * Another fix 🔧 * Merge * FIXES * Add another missing REQUIRE
kapenacunmin · Mar 21, 2024 · 0a15637 · 0a15637
1 parent b4949f7
commit 0a15637
Show file tree

Hide file tree

Showing 164 changed files with 647 additions and 642 deletions.
diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn
@@ -1,7 +1,7 @@
 {:config-paths ["macros"]
  :linters
  {:aliased-namespace-symbol     {:level :warning}
-  :invalid-arity                {:skip-args [metabase.mbql.util.match/match]} ; TODO (cam): can we fix these?
+  :invalid-arity                {:skip-args [metabase.lib.util.match/match]} ; TODO (cam): can we fix these?
   :keyword-binding              {:level :warning}
   :main-without-gen-class       {:level :warning}
   :misplaced-docstring          {:level :warning}
@@ -47,14 +47,10 @@
     instaparse.core/transform
     (metabase.async.streaming-response/streaming-response)
     (metabase.driver.druid.query-processor-test/druid-query-returning-rows)
-    (metabase.mbql.util.match/match)
-    (metabase.mbql.util.match/match-one)
-    (metabase.mbql.util.match/replace)
-    (metabase.mbql.util.match/replace-in)
-    (metabase.mbql.util/match)
-    (metabase.mbql.util/match-one)
-    (metabase.mbql.util/replace)
-    (metabase.mbql.util/replace-in)
+    (metabase.lib.util.match/match)
+    (metabase.lib.util.match/match-one)
+    (metabase.lib.util.match/replace)
+    (metabase.lib.util.match/replace-in)
     (metabase.query-processor.middleware.cache-backend.interface/with-cached-results)
     (metabase.util.regex/rx [opt])
     (metabase.util/prog1 [<>])
@@ -262,10 +258,10 @@
     metabase.email.messages                                       messages
     metabase.formatter                                            formatter
     metabase.http-client                                          client
+    metabase.legacy-mbql.normalize                                mbql.normalize
+    metabase.legacy-mbql.schema                                   mbql.s
+    metabase.legacy-mbql.util                                     mbql.u
     metabase.lib.util                                             lib.util
-    metabase.mbql.normalize                                       mbql.normalize
-    metabase.mbql.schema                                          mbql.s
-    metabase.mbql.util                                            mbql.u
     metabase.models.activity                                      activity
     metabase.models.application-permissions-revision              a-perm-revision
     metabase.models.bookmark                                      bookmark
@@ -524,13 +520,13 @@
    metabase.db.schema-migrations-test.impl/test-migrations                                                                   hooks.metabase.db.schema-migrations-test.impl/test-migrations
    metabase.dashboard-subscription-test/with-link-card-fixture-for-dashboard                                                 hooks.common/let-second
    metabase.driver.bigquery-cloud-sdk-test/calculate-bird-scarcity                                                           hooks.metabase.query-processor-test.expressions-test/calculate-bird-scarcity
+   metabase.legacy-mbql.schema.macros/defclause                                                                              hooks.metabase.legacy-mbql.schemas.macros/defclause
    metabase.lib.schema.mbql-clause/define-mbql-clause                                                                        hooks.metabase.lib.schema.mbql-clause/define-mbql-clause
    metabase.lib.schema.mbql-clause/define-catn-mbql-clause                                                                   hooks.metabase.lib.schema.mbql-clause/define-mbql-clause
    metabase.lib.schema.mbql-clause/define-tuple-mbql-clause                                                                  hooks.metabase.lib.schema.mbql-clause/define-mbql-clause
    metabase.lib.test-util.macros/$ids                                                                                        hooks.metabase.test.data/$ids
    metabase.lib.test-util.macros/mbql-query                                                                                  hooks.metabase.test.data/mbql-query
    metabase.lib.test-util.macros/with-testing-against-standard-queries                                                       hooks.metabase.lib.test-util.macros/with-testing-against-standard-queries
-   metabase.mbql.schema.macros/defclause                                                                                     hooks.metabase.mbql.schemas.macros/defclause
    metabase.models.collection-test/with-collection-hierarchy                                                                 hooks.common/let-one-with-optional-value
    metabase.models.collection-test/with-personal-and-impersonal-collections                                                  hooks.common/with-two-bindings
    metabase.models.dashboard-test/with-dash-in-collection                                                                    hooks.common/with-three-bindings

diff --git a/...do/hooks/metabase/mbql/schemas/macros.clj → ...s/metabase/legacy_mbql/schemas/macros.clj b/...do/hooks/metabase/mbql/schemas/macros.clj → ...s/metabase/legacy_mbql/schemas/macros.clj
@@ -1,5 +1,6 @@
-(ns hooks.metabase.mbql.schemas.macros
-  (:require [clj-kondo.hooks-api :as hooks]))
+(ns hooks.metabase.legacy-mbql.schemas.macros
+  (:require
+   [clj-kondo.hooks-api :as hooks]))
 
 (defn- unwrap-defclause-clause-name
   "The `clause-name-form` can be either a plain symbol, or a vector like `[var-name mbql-clause-name]`. For the vector

diff --git a/.cljfmt.edn b/.cljfmt.edn
@@ -7,7 +7,7 @@
   defprotocol+                                                     [[:block 1] [:inner 1]]
   potemkin/defprotocol+                                            [[:block 1] [:inner 1]]
   let-404                                                          [[:block 1]]
-  metabase.mbql.util/match-one                                     [[:inner 0]]
+  metabase.legacy-mbql.util/match-one                              [[:inner 0]]
   merge-with                                                       [[:inner 0]]
   metabase.test/dataset                                            [[:inner 0]]
   metabase.test/formatted-rows                                     [[:inner 0]]

diff --git a/.dir-locals.el b/.dir-locals.el
@@ -45,26 +45,26 @@
   (eval . (put-clojure-indent 'c/step 1))
   (eval . (put-clojure-indent 'impl/test-migrations 2))
   (eval . (put-clojure-indent 'let-404 0))
+  (eval . (put-clojure-indent 'lib.schema.match/match 1))
+  (eval . (put-clojure-indent 'lib.schema.match/match-one 1))
+  (eval . (put-clojure-indent 'lib.schema.match/replace 1))
+  (eval . (put-clojure-indent 'lib.schema.match/replace-in 2))
+  (eval . (put-clojure-indent 'lib.util.match/replace 1))
   (eval . (put-clojure-indent 'macros/case 0))
   (eval . (put-clojure-indent 'match 1))
-  (eval . (put-clojure-indent 'mbql.match/match 1))
-  (eval . (put-clojure-indent 'mbql.match/match-one 1))
-  (eval . (put-clojure-indent 'mbql.match/replace 1))
-  (eval . (put-clojure-indent 'mbql.match/replace-in 2))
-  (eval . (put-clojure-indent 'mbql.u/replace 1))
   (eval . (put-clojure-indent 'mi/define-methods '(:form)))
   (eval . (put-clojure-indent 'mt/dataset 1))
   (eval . (put-clojure-indent 'mt/format-rows-by 1))
   (eval . (put-clojure-indent 'mt/query 1))
-  (eval . (put-clojure-indent 'mt/test-drivers 1))
   (eval . (put-clojure-indent 'mt/test-driver 1))
+  (eval . (put-clojure-indent 'mt/test-drivers 1))
   (eval . (put-clojure-indent 'prop/for-all 1))
   (eval . (put-clojure-indent 'qp.streaming/streaming-response 1))
+  (eval . (put-clojure-indent 'tc/quick-check 1))
   (eval . (put-clojure-indent 'u/profile 1))
   (eval . (put-clojure-indent 'u/prog1 1))
   (eval . (put-clojure-indent 'u/select-keys-when 1))
   (eval . (put-clojure-indent 'with-meta '(:form)))
-  (eval . (put-clojure-indent 'tc/quick-check 1))
   ;; these ones have to be done with `define-clojure-indent' for now because of upstream bug
   ;; https://github.com/clojure-emacs/clojure-mode/issues/600 once that's resolved we should use `put-clojure-indent'
   ;; instead. Please don't add new entries unless they don't work with `put-clojure-indent'

diff --git a/deps.edn b/deps.edn
@@ -684,7 +684,7 @@
                       metabase-enterprise.advanced-permissions.models.permissions.block-permissions-test
                       metabase-enterprise.sandbox.query-processor.middleware.row-level-restrictions-test]}}
 
-  ;; test all MBQL related stuff: MLv2, the legacy shared `metabase.mbql` code, and the QP
+  ;; test all MBQL related stuff: MLv2, the legacy shared `metabase.legacy-mbql` code, and the QP
   ;;
   ;;    clj -X:dev:ee:ee-dev:test:test/mbql
   :test/mbql

diff --git a/dev/src/dev/debug_qp.clj b/dev/src/dev/debug_qp.clj
@@ -7,8 +7,9 @@
    [medley.core :as m]
    [metabase.db :as mdb]
    [metabase.driver :as driver]
-   [metabase.mbql.normalize :as mbql.normalize]
-   [metabase.mbql.util :as mbql.u]
+   [metabase.legacy-mbql.normalize :as mbql.normalize]
+   [metabase.legacy-mbql.util :as mbql.u]
+   [metabase.lib.util.match :as lib.util.match]
    [metabase.models.field :refer [Field]]
    [metabase.models.table :refer [Table]]
    [metabase.util :as u]
@@ -116,7 +117,7 @@
                        (symbol (format "#_\"%s.%s\"" field-name table-name)))))
                  (field-id->name-form [field-id]
                    (list 'do (add-name-to-field-id field-id) field-id))]
-           (mbql.u/replace form
+           (lib.util.match/replace form
              [:field (id :guard pos-int?) opts]
              [:field id (add-name-to-field-id id) (cond-> opts
                                                     (pos-int? (:source-field opts))
@@ -255,7 +256,7 @@
         coll))
 
 (defn- can-symbolize? [x]
-  (mbql.u/match-one x
+  (lib.util.match/match-one x
     (_ :guard string?)
     (not (re-find #"\s+" x))
 
@@ -283,7 +284,7 @@
 
 (defn- expand [form table]
   (try
-    (mbql.u/replace form
+    (lib.util.match/replace form
       ([:field (id :guard pos-int?) nil] :guard can-symbolize?)
       (let [[table-name field-name] (field-and-table-name id)
             field-name              (some-> field-name u/lower-case-en)
@@ -339,10 +340,10 @@
                       e)))))
 
 (defn- no-$ [x]
-  (mbql.u/replace x [::$ & args] (into [::no-$] args)))
+  (lib.util.match/replace x [::$ & args] (into [::no-$] args)))
 
 (defn- symbolize [form]
-  (mbql.u/replace form
+  (lib.util.match/replace form
     [::-> x y]
     (symbol (format "%s->%s" (symbolize x) (str/replace (symbolize y) #"^\$" "")))
 

diff --git a/docs/developers-guide/driver-changelog.md b/docs/developers-guide/driver-changelog.md
@@ -4,6 +4,11 @@ title: Driver interface changelog
 
 # Driver Interface Changelog
 
+## Metabase 0.50.0
+
+- The Metabase `metabase.mbql.*` namespaces have been moved to `metabase.legacy-mbql.*`. You probably didn't need to
+  use these namespaces in your driver, but if you did, please update them.
+
 ## Metabase 0.49.1
 
 - Another driver feature has been added: `describe-fields`. If a driver opts-in to supporting this feature, The

diff --git a/enterprise/backend/src/metabase_enterprise/sandbox/api/table.clj b/enterprise/backend/src/metabase_enterprise/sandbox/api/table.clj
@@ -4,7 +4,7 @@
    [metabase-enterprise.sandbox.api.util :as sandbox.api.util]
    [metabase.api.common :as api]
    [metabase.api.table :as api.table]
-   [metabase.mbql.util :as mbql.u]
+   [metabase.lib.util.match :as lib.util.match]
    [metabase.models.card :refer [Card]]
    [metabase.models.data-permissions :as data-perms]
    [metabase.models.interface :as mi]
@@ -38,7 +38,7 @@
 
 (mu/defn ^:private query->fields-ids :- [:maybe [:sequential :int]]
   [{{{:keys [fields]} :query} :dataset_query} :- [:maybe :map]]
-  (mbql.u/match fields [:field (id :guard integer?) _] id))
+  (lib.util.match/match fields [:field (id :guard integer?) _] id))
 
 (defn- maybe-filter-fields [table query-metadata-response]
   ;; If we have sandboxed permissions and the associated GTAP limits the fields returned, we need make sure the

diff --git a/enterprise/backend/src/metabase_enterprise/sandbox/models/group_table_access_policy.clj b/enterprise/backend/src/metabase_enterprise/sandbox/models/group_table_access_policy.clj
@@ -7,7 +7,7 @@
   (:require
    [medley.core :as m]
    [metabase.config :as config]
-   [metabase.mbql.normalize :as mbql.normalize]
+   [metabase.legacy-mbql.normalize :as mbql.normalize]
    [metabase.models.card :refer [Card]]
    [metabase.models.data-permissions :as data-perms]
    [metabase.models.database :as database]

diff --git a/enterprise/backend/src/metabase_enterprise/sandbox/models/params/field_values.clj b/enterprise/backend/src/metabase_enterprise/sandbox/models/params/field_values.clj
@@ -8,7 +8,7 @@
    [metabase-enterprise.sandbox.query-processor.middleware.row-level-restrictions
     :as row-level-restrictions]
    [metabase.api.common :as api]
-   [metabase.mbql.util :as mbql.u]
+   [metabase.lib.util.match :as lib.util.match]
    [metabase.models :refer [Field PermissionsGroupMembership]]
    [metabase.models.field :as field]
    [metabase.models.field-values :as field-values]
@@ -76,7 +76,7 @@
          (into {} (for [[k v] attribute_remappings
                         ;; get attribute that map to fields of the same table
                         :when (contains? field-ids
-                                         (mbql.u/match-one v [:dimension [:field field-id _]] field-id))]
+                                         (lib.util.match/match-one v [:dimension [:field field-id _]] field-id))]
                     {k (get login-attributes k)})))])))
 
 (defenterprise field-id->field-values-for-current-user

diff --git a/...d/src/metabase_enterprise/sandbox/query_processor/middleware/column_level_perms_check.clj b/...d/src/metabase_enterprise/sandbox/query_processor/middleware/column_level_perms_check.clj
@@ -2,13 +2,13 @@
   (:require
    [medley.core :as m]
    [metabase.api.common :refer [*current-user-id*]]
-   [metabase.mbql.util :as mbql.u]
+   [metabase.lib.util.match :as lib.util.match]
    [metabase.public-settings.premium-features :refer [defenterprise]]
    [metabase.util.i18n :refer [trs tru]]
    [metabase.util.log :as log]))
 
 (defn- field-ids [form]
-  (set (mbql.u/match form
+  (set (lib.util.match/match form
          [:field (id :guard integer?) _]
          id)))
 

diff --git a/...end/src/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions.clj b/...end/src/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions.clj
@@ -9,10 +9,10 @@
    [metabase-enterprise.sandbox.models.group-table-access-policy :as gtap]
    [metabase.api.common :as api :refer [*current-user* *current-user-id*]]
    [metabase.db :as mdb]
+   [metabase.legacy-mbql.schema :as mbql.s]
    [metabase.lib.metadata :as lib.metadata]
    [metabase.lib.metadata.protocols :as lib.metadata.protocols]
-   [metabase.mbql.schema :as mbql.s]
-   [metabase.mbql.util :as mbql.u]
+   [metabase.lib.util.match :as lib.util.match]
    [metabase.models.card :refer [Card]]
    [metabase.models.query.permissions :as query-perms]
    [metabase.public-settings.premium-features :refer [defenterprise]]
@@ -35,7 +35,7 @@
 ;;; +----------------------------------------------------------------------------------------------------------------+
 
 (defn- all-table-ids [m]
-  (into #{} cat (mbql.u/match m
+  (into #{} cat (lib.util.match/match m
                   (_ :guard (every-pred map? :source-table (complement ::gtap?)))
                   (let [recursive-ids (all-table-ids (dissoc &match :source-table))]
                     (cons (:source-table &match) recursive-ids)))))
@@ -81,7 +81,7 @@
   "If the `:target` of a parameter contains a `:field` clause, return the base type corresponding to the Field it
   references. Otherwise returns `nil`."
   [[_ target-field-clause]]
-  (when-let [field-id (mbql.u/match-one target-field-clause [:field (field-id :guard integer?) _] field-id)]
+  (when-let [field-id (lib.util.match/match-one target-field-clause [:field (field-id :guard integer?) _] field-id)]
     (:base-type (lib.metadata.protocols/field (qp.store/metadata-provider) field-id))))
 
 (defn- attr-value->param-value
@@ -235,7 +235,7 @@
   [{table-id :table_id, attribute-remappings :attribute_remappings}]
   (->>
    (for [target-field-clause (vals attribute-remappings)]
-     (mbql.u/match-one target-field-clause
+     (lib.util.match/match-one target-field-clause
        [:field (field-id :guard integer?) _]
        (:table-id (lib.metadata.protocols/field (qp.store/metadata-provider) field-id))))
    (cons table-id)
@@ -292,7 +292,7 @@
   from their GTAPs."
   [m table-id->gtap]
   ;; replace maps that have `:source-table` key and a matching entry in `table-id->gtap`, but do not have `::gtap?` key
-  (mbql.u/replace m
+  (lib.util.match/replace m
     (_ :guard (every-pred map? (complement ::gtap?) :source-table #(get table-id->gtap (:source-table %))))
     (let [updated             (apply-gtap &match (get table-id->gtap (:source-table &match)))
           ;; now recursively apply gtaps anywhere else they might exist at this level, e.g. `:joins`
@@ -301,7 +301,7 @@
                                (apply-gtaps (dissoc updated :source-table :source-query) table-id->gtap))]
       ;; add a `::gtap?` key next to every `:source-table` key so when we do a second pass after adding JOINs they
       ;; don't get processed again
-      (mbql.u/replace recursively-updated
+      (lib.util.match/replace recursively-updated
         (_ :guard (every-pred map? :source-table))
         (assoc &match ::gtap? true)))))
 

diff --git a/enterprise/backend/src/metabase_enterprise/serialization/load.clj b/enterprise/backend/src/metabase_enterprise/serialization/load.clj
@@ -11,8 +11,9 @@
    [metabase-enterprise.serialization.upsert :refer [maybe-upsert-many!]]
    [metabase.config :as config]
    [metabase.db :as mdb]
-   [metabase.mbql.normalize :as mbql.normalize]
-   [metabase.mbql.util :as mbql.u]
+   [metabase.legacy-mbql.normalize :as mbql.normalize]
+   [metabase.legacy-mbql.util :as mbql.u]
+   [metabase.lib.util.match :as lib.util.match]
    [metabase.models.card :refer [Card]]
    [metabase.models.collection :refer [Collection]]
    [metabase.models.dashboard :refer [Dashboard]]
@@ -146,7 +147,7 @@
 
 (defn- mbql-fully-qualified-names->ids*
   [entity]
-  (mbql.u/replace entity
+  (lib.util.match/replace entity
     ;; handle legacy `:field-id` forms encoded prior to 0.39.0
     ;; and also *current* expresion forms used in parameter mapping dimensions
     ;; example relevant clause - [:dimension [:fk-> [:field-id 1] [:field-id 2]]]

diff --git a/enterprise/backend/src/metabase_enterprise/serialization/serialize.clj b/enterprise/backend/src/metabase_enterprise/serialization/serialize.clj
@@ -4,9 +4,9 @@
    [clojure.string :as str]
    [medley.core :as m]
    [metabase-enterprise.serialization.names :refer [fully-qualified-name]]
+   [metabase.legacy-mbql.normalize :as mbql.normalize]
    [metabase.lib.schema.id :as lib.schema.id]
-   [metabase.mbql.normalize :as mbql.normalize]
-   [metabase.mbql.util :as mbql.u]
+   [metabase.lib.util.match :as lib.util.match]
    [metabase.models.card :refer [Card]]
    [metabase.models.dashboard :refer [Dashboard]]
    [metabase.models.dashboard-card :refer [DashboardCard]]
@@ -48,7 +48,7 @@
   [mbql]
   (-> mbql
       mbql.normalize/normalize-tokens
-      (mbql.u/replace
+      (lib.util.match/replace
         ;; `integer?` guard is here to make the operation idempotent
         [:field (id :guard integer?) opts]
         [:field (fully-qualified-name Field id) (mbql-id->fully-qualified-name opts)]
@@ -71,7 +71,7 @@
 
 (defn- ids->fully-qualified-names
   [entity]
-  (mbql.u/replace entity
+  (lib.util.match/replace entity
     mbql-entity-reference?
     (mbql-id->fully-qualified-name &match)
 

diff --git a/...st/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions_test.clj b/...st/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions_test.clj
@@ -12,8 +12,8 @@
    [metabase.api.common :as api]
    [metabase.driver :as driver]
    [metabase.driver.sql.query-processor :as sql.qp]
-   [metabase.mbql.normalize :as mbql.normalize]
-   [metabase.mbql.util :as mbql.u]
+   [metabase.legacy-mbql.normalize :as mbql.normalize]
+   [metabase.lib.util.match :as lib.util.match]
    [metabase.models :refer [Card Collection Field Table]]
    [metabase.models.data-permissions :as data-perms]
    [metabase.models.permissions :as perms]
@@ -165,7 +165,7 @@
 
 ;; TODO -- #19754 adds [[mt/remove-source-metadata]] that can be used here (once it gets merged)
 (defn- remove-metadata [m]
-  (mbql.u/replace m
+  (lib.util.match/replace m
     (_ :guard (every-pred map? :source-metadata))
     (remove-metadata (dissoc &match :source-metadata))))
 

diff --git a/frontend/src/metabase-lib/v1/queries/utils/normalize.js b/frontend/src/metabase-lib/v1/queries/utils/normalize.js
@@ -1 +1 @@
-export { normalize } from "cljs/metabase.mbql.js";
+export { normalize } from "cljs/metabase.legacy_mbql.js";
diff --git a/frontend/test/metabase-lib/lib/normalize.unit.spec.js b/frontend/test/metabase-lib/lib/normalize.unit.spec.js
@@ -1,4 +1,4 @@
-import { normalize } from "cljs/metabase.mbql.js";
+import { normalize } from "cljs/metabase.legacy_mbql.js";
 
 // This test is here mostly to make sure the shared CLJS lib works correctly.
 

diff --git a/...les/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk/query_processor.clj b/...les/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk/query_processor.clj
@@ -7,14 +7,13 @@
    [metabase.driver.bigquery-cloud-sdk.common :as bigquery.common]
    [metabase.driver.common :as driver.common]
    [metabase.driver.sql :as driver.sql]
-   [metabase.driver.sql.parameters.substitution
-    :as sql.params.substitution]
+   [metabase.driver.sql.parameters.substitution :as sql.params.substitution]
    [metabase.driver.sql.query-processor :as sql.qp]
    [metabase.driver.sql.util :as sql.u]
    [metabase.driver.sql.util.unprepare :as unprepare]
+   [metabase.legacy-mbql.util :as mbql.u]
    [metabase.lib.metadata :as lib.metadata]
    [metabase.lib.schema.metadata :as lib.schema.metadata]
-   [metabase.mbql.util :as mbql.u]
    [metabase.models.setting :as setting]
    [metabase.query-processor.error-type :as qp.error-type]
    [metabase.query-processor.store :as qp.store]
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		export { normalize } from "cljs/metabase.mbql.js";
		export { normalize } from "cljs/metabase.legacy_mbql.js";