Skip to content
Snippets Groups Projects
Unverified Commit 435fada0 authored by Cam Saul's avatar Cam Saul Committed by GitHub
Browse files

MLv2 schema should enforce distinct breakouts and fields (#33512)

* MLv2 schema should enforce  distinct breakouts and fields #32489

* Enforce distinct breakouts and fields (#32489)
parent f8ba4959
No related branches found
No related tags found
No related merge requests found
......@@ -6,7 +6,9 @@
Some primitives below are duplicated from [[metabase.util.malli.schema]] since that's not `.cljc`. Other stuff is
copied from [[metabase.mbql.schema]] so this can exist completely independently; hopefully at some point in the
future we can deprecate that namespace and eventually do away with it entirely."
(:refer-clojure :exclude [ref])
(:require
[metabase.lib.options :as lib.options]
[metabase.lib.schema.aggregation :as aggregation]
[metabase.lib.schema.common :as common]
[metabase.lib.schema.expression :as expression]
......@@ -50,13 +52,30 @@
;; `:native` key), but their definition lives under this key.
[:template-tags {:optional true} [:ref ::template-tag/template-tag-map]]])
(mr/def ::breakout
[:ref ::ref/ref])
(defn- distinct-refs? [refs]
(or
(< (count refs) 2)
(apply
distinct?
(for [ref refs]
(lib.options/update-options ref dissoc :lib/uuid)))))
(mr/def ::breakouts
[:sequential {:min 1} [:ref ::ref/ref]])
[:and
[:sequential {:min 1} ::breakout]
[:fn
{:error/message "Breakouts must be distinct"}
distinct-refs?]])
;;; TODO -- `:fields` is supposed to be distinct (ignoring UUID), e.g. you can't have `[:field {} 1]` in there
;;; twice. (#32489)
(mr/def ::fields
[:sequential {:min 1} [:ref ::ref/ref]])
[:and
[:sequential {:min 1} [:ref ::ref/ref]]
[:fn
{:error/message ":fields must be distinct"}
distinct-refs?]])
;; this is just for enabling round-tripping filters with named segment references
(mr/def ::filterable
......
......@@ -197,3 +197,19 @@
{:lib/type :mbql.stage/mbql
:fields [[:field {:lib/uuid (str (random-uuid)), :join-alias "A"} 1]]}]
nil))
(deftest ^:parallel enforce-distinct-breakouts-and-fields-test
(let [duplicate-refs [[:field {:lib/uuid "00000000-0000-0000-0000-000000000000"} 1]
[:field {:lib/uuid "00000000-0000-0000-0000-000000000001"} 1]]]
(testing #'lib.schema/distinct-refs?
(is (not (#'lib.schema/distinct-refs? duplicate-refs))))
(testing "breakouts/fields schemas"
(are [schema error] (= error
(me/humanize (mc/explain schema duplicate-refs)))
::lib.schema/breakouts ["Breakouts must be distinct"]
::lib.schema/fields [":fields must be distinct"]))
(testing "stage schema"
(are [k error] (= error
(me/humanize (mc/explain ::lib.schema/stage {:lib/type :mbql.stage/mbql, k duplicate-refs})))
:breakout {:breakout ["Breakouts must be distinct"]}
:fields {:fields [":fields must be distinct"]}))))
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment