From 8abce7995b64d7faf5c6738a6a5eb25b2ee4335c Mon Sep 17 00:00:00 2001
From: metamben <103100869+metamben@users.noreply.github.com>
Date: Tue, 28 Mar 2023 08:47:02 +0300
Subject: [PATCH] MLv2 new expression syntax (#29565)
---
src/metabase/lib/common.cljc | 40 ++--
src/metabase/lib/core.cljc | 16 +-
src/metabase/lib/dispatch.cljc | 7 +-
src/metabase/lib/expression.cljc | 7 +-
src/metabase/lib/filter.cljc | 163 +++++----------
src/metabase/lib/join.cljc | 8 +
src/metabase/lib/schema/common.cljc | 6 +
test/metabase/lib/aggregation_test.cljc | 47 +++--
test/metabase/lib/expression_test.cljc | 7 +-
test/metabase/lib/filter_test.cljc | 264 ++++++++----------------
test/metabase/lib/join_test.cljc | 23 ++-
test/metabase/lib/query_test.cljc | 2 +-
test/metabase/lib/util_test.cljc | 2 +-
13 files changed, 248 insertions(+), 344 deletions(-)
diff --git a/src/metabase/lib/common.cljc b/src/metabase/lib/common.cljc
index 4877fe18ca7..e11b85e1760 100644
--- a/src/metabase/lib/common.cljc
+++ b/src/metabase/lib/common.cljc
@@ -4,10 +4,20 @@
[metabase.lib.field :as lib.field]
#_{:clj-kondo/ignore [:unused-namespace]}
[metabase.lib.options :as lib.options]
+ [metabase.lib.schema.common :as schema.common]
#_{:clj-kondo/ignore [:unused-namespace]}
[metabase.util.malli :as mu])
#?(:cljs (:require-macros [metabase.lib.common])))
+(mu/defn external-op :- [:maybe ::schema.common/external-op]
+ "Convert the internal operator `clause` to the external format."
+ [[operator options :as clause]]
+ (when clause
+ {:operator (cond-> operator
+ (keyword? operator) name)
+ :options options
+ :args (subvec clause 2)}))
+
(defmulti ->op-arg
"Ensures that clause arguments are properly unwrapped"
{:arglists '([query stage-number x])}
@@ -15,15 +25,17 @@
(lib.dispatch/dispatch-value x)))
(defmethod ->op-arg :default
- [query stage-number x]
- (if (vector? x)
- (mapv #(->op-arg query stage-number %) x)
- x))
+ [_query _stage-number x]
+ x)
(defmethod ->op-arg :metadata/field
[query stage-number field-metadata]
(lib.field/field query stage-number field-metadata))
+(defmethod ->op-arg :lib/external-op
+ [query stage-number {:keys [operator options args] :or {options {}}}]
+ (->op-arg query stage-number (lib.options/ensure-uuid (into [(keyword operator) options] args))))
+
(defmethod ->op-arg :dispatch-type/fn
[query stage-number f]
(->op-arg query stage-number (f query stage-number)))
@@ -38,26 +50,28 @@
(every? #(not-any? #{'query 'stage-number} %) argvecs)]}
(let [fn-rename #(name (get {'/ 'div} % %))]
`(do
- (mu/defn ~(symbol (str (fn-rename op-name) "-clause")) :- ~(keyword "mbql.clause" (name op-name))
+ (mu/defn ~(symbol (str (fn-rename op-name) "-clause")) :- :metabase.lib.schema.common/external-op
~(format "Create a standalone clause of type `%s`." (name op-name))
~@(for [argvec argvecs
:let [arglist-expr (if (contains? (set argvec) '&)
(cons `list* (remove #{'&} argvec))
argvec)]]
`([~'query ~'stage-number ~@argvec]
- (-> (into [~(keyword op-name)]
- (map (fn [~'arg]
- (->op-arg ~'query ~'stage-number ~'arg)))
- ~arglist-expr)
- lib.options/ensure-uuid))))
+ {:operator ~(keyword op-name)
+ :args (mapv (fn [~'arg]
+ (->op-arg ~'query ~'stage-number ~'arg))
+ ~arglist-expr)})))
(mu/defn ~op-name :- fn?
~(format "Create a closure of clause of type `%s`." (name op-name))
~@(for [argvec argvecs
- :let [arglist-expr (if (contains? (set argvec) '&)
+ :let [varargs? (contains? (set argvec) '&)
+ arglist-expr (if varargs?
(filterv (complement #{'&}) argvec)
- (conj argvec []))]]
+ argvec)]]
`([~@argvec]
(fn ~(symbol (str (fn-rename op-name) "-closure"))
[~'query ~'stage-number]
- (apply ~(symbol (str (fn-rename op-name) "-clause")) ~'query ~'stage-number ~@arglist-expr)))))))))
+ ~(cond->> (concat [(symbol (str (fn-rename op-name) "-clause")) 'query 'stage-number]
+ arglist-expr)
+ varargs? (cons `apply))))))))))
diff --git a/src/metabase/lib/core.cljc b/src/metabase/lib/core.cljc
index e202e9b0e99..a148d11410e 100644
--- a/src/metabase/lib/core.cljc
+++ b/src/metabase/lib/core.cljc
@@ -118,21 +118,7 @@
starts-with ends-with
contains does-not-contain
time-interval
- segment
- ->and
- ->or
- ->not
- ->= ->!=
- ->< -><=
- ->> ->>=
- ->between
- ->inside
- ->is-null ->not-null
- ->is-empty ->not-empty
- ->starts-with ->ends-with
- ->contains ->does-not-contain
- ->time-interval
- ->segment]
+ segment]
[lib.join
join
join-clause
diff --git a/src/metabase/lib/dispatch.cljc b/src/metabase/lib/dispatch.cljc
index 552c6fa305f..9987893bd4a 100644
--- a/src/metabase/lib/dispatch.cljc
+++ b/src/metabase/lib/dispatch.cljc
@@ -21,5 +21,10 @@
;; of [[toucan2.core/model]]?
(or (mbql-clause-type x)
(when (map? x)
- (:lib/type x))
+ (if (and (or (keyword? (:operator x))
+ (string? (:operator x)))
+ (vector? (:args x))
+ (map? (:options x {})))
+ :lib/external-op
+ (:lib/type x)))
(u/dispatch-type-keyword x)))
diff --git a/src/metabase/lib/expression.cljc b/src/metabase/lib/expression.cljc
index 556a2b5a100..e173572fefe 100644
--- a/src/metabase/lib/expression.cljc
+++ b/src/metabase/lib/expression.cljc
@@ -23,12 +23,17 @@
stage-number :- :int
expression-name :- ::lib.schema.common/non-blank-string]
(let [stage (lib.util/query-stage query stage-number)]
- (or (get-in stage [:expressions expression-name])
+ (or (some-> (get-in stage [:expressions expression-name])
+ lib.common/external-op)
(throw (ex-info (i18n/tru "No expression named {0}" (pr-str expression-name))
{:expression-name expression-name
:query query
:stage-number stage-number})))))
+(defmethod lib.schema.expression/type-of* :lib/external-op
+ [{:keys [operator options args] :or {options {}}}]
+ (lib.schema.expression/type-of* (into [(keyword operator) options] args)))
+
(defmethod lib.metadata.calculation/metadata :expression
[query stage-number [_expression opts expression-name, :as expression-ref]]
(let [expression (resolve-expression query stage-number expression-name)]
diff --git a/src/metabase/lib/filter.cljc b/src/metabase/lib/filter.cljc
index 289722ab9d0..b2ea06bdc3a 100644
--- a/src/metabase/lib/filter.cljc
+++ b/src/metabase/lib/filter.cljc
@@ -1,13 +1,12 @@
(ns metabase.lib.filter
(:refer-clojure :exclude [filter and or not = < <= > ->> >= not-empty case])
(:require
- [metabase.lib.dispatch :as lib.dispatch]
- [metabase.lib.field :as lib.field]
+ [metabase.lib.common :as lib.common]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
[metabase.lib.options :as lib.options]
[metabase.lib.schema]
- [metabase.lib.schema.expression :as schema.expression]
+ [metabase.lib.schema.common :as schema.common]
[metabase.lib.temporal-bucket :as lib.temporal-bucket]
[metabase.lib.util :as lib.util]
[metabase.shared.util.i18n :as i18n]
@@ -167,129 +166,70 @@
(lib.metadata.calculation/display-name query stage-number expr)
(lib.temporal-bucket/interval->i18n n unit)))
-(defmulti ^:private ->filter-arg
- {:arglists '([query stage-number x])}
- (fn [_query _stage-number x]
- (lib.dispatch/dispatch-value x)))
-
-(defmethod ->filter-arg :default
- [_query _stage-number x]
- x)
-
-(defmethod ->filter-arg :metadata/field
- [query stage-number field-metadata]
- (lib.field/field query stage-number field-metadata))
-
-(defmethod ->filter-arg :dispatch-type/fn
- [query stage-number f]
- (->filter-arg query stage-number (f query stage-number)))
-
-#?(:clj
- (defmacro ^:private deffilter
- [filter-name argvec]
- {:pre [(symbol? filter-name)
- (vector? argvec) (every? symbol? argvec)
- (not-any? #{'query 'stage-number} argvec)]}
- (let [filter-name-str (name filter-name)
- vararg? (.contains ^java.util.Collection argvec '&)
- args (remove #{'&} argvec)
- arglist-expr (if vararg?
- (cons 'list* args)
- argvec)]
- `(do
- (mu/defn ~filter-name :- ~(keyword "mbql.clause" filter-name-str)
- ~(format "Create a filter clause of type `%s`." filter-name-str)
- [~'query ~'stage-number ~@argvec]
- (-> (into [~(keyword filter-name)]
- (map (fn [~'arg]
- (->filter-arg ~'query ~'stage-number ~'arg)))
- ~arglist-expr)
- metabase.lib.options/ensure-uuid))
-
- (mu/defn ~(symbol (str "->" filter-name-str)) :- fn?
- ~(format "Return function creating a filter clause of type `%s`." filter-name-str)
- ~argvec
- (fn [~'query ~'stage-number]
- ~(cond->> (concat [filter-name 'query 'stage-number] args)
- vararg? (cons `apply))))))))
-
-(metabase.lib.filter/deffilter and [x y & more])
-(metabase.lib.filter/deffilter or [x y & more])
-(metabase.lib.filter/deffilter not [x])
-(metabase.lib.filter/deffilter = [x y & more])
-(metabase.lib.filter/deffilter != [x y & more])
-(metabase.lib.filter/deffilter < [x y])
-(metabase.lib.filter/deffilter <= [x y])
-(metabase.lib.filter/deffilter > [x y])
-(metabase.lib.filter/deffilter >= [x y])
-(metabase.lib.filter/deffilter between [x lower upper])
-(metabase.lib.filter/deffilter inside [lat lon lat-max lon-min lat-min lon-max])
-(metabase.lib.filter/deffilter is-null [x])
-(metabase.lib.filter/deffilter not-null [x])
-(metabase.lib.filter/deffilter is-empty [x])
-(metabase.lib.filter/deffilter not-empty [x])
-(metabase.lib.filter/deffilter starts-with [whole part])
-(metabase.lib.filter/deffilter ends-with [whole part])
-(metabase.lib.filter/deffilter contains [whole part])
-(metabase.lib.filter/deffilter does-not-contain [whole part])
-(metabase.lib.filter/deffilter time-interval [x amount unit])
-(metabase.lib.filter/deffilter segment [segment-id])
-
-(defmulti ^:private ->filter-clause
- {:arglists '([query stage-number x])}
- (fn [_query _stage-number x]
- (lib.dispatch/dispatch-value x)))
-
-(defmethod ->filter-clause :default
- [query stage-number x]
- (if (vector? x)
- (-> (mapv #(clojure.core/->> %
- (->filter-arg query stage-number)
- (->filter-clause query stage-number)) x)
- lib.options/ensure-uuid)
- x))
-
-(defmethod ->filter-clause :dispatch-type/fn
- [query stage-number f]
- (->filter-clause query stage-number (f query stage-number)))
+(lib.common/defop and [x y & more])
+(lib.common/defop or [x y & more])
+(lib.common/defop not [x])
+(lib.common/defop = [x y & more])
+(lib.common/defop != [x y & more])
+(lib.common/defop < [x y])
+(lib.common/defop <= [x y])
+(lib.common/defop > [x y])
+(lib.common/defop >= [x y])
+(lib.common/defop between [x lower upper])
+(lib.common/defop inside [lat lon lat-max lon-min lat-min lon-max])
+(lib.common/defop is-null [x])
+(lib.common/defop not-null [x])
+(lib.common/defop is-empty [x])
+(lib.common/defop not-empty [x])
+(lib.common/defop starts-with [whole part])
+(lib.common/defop ends-with [whole part])
+(lib.common/defop contains [whole part])
+(lib.common/defop does-not-contain [whole part])
+(lib.common/defop time-interval [x amount unit])
+(lib.common/defop segment [segment-id])
(mu/defn filter :- :metabase.lib.schema/query
"Sets `boolean-expression` as a filter on `query`."
- ([query boolean-expression]
- (metabase.lib.filter/filter query -1 boolean-expression))
+ ([query :- :metabase.lib.schema/query
+ boolean-expression]
+ (metabase.lib.filter/filter query nil boolean-expression))
- ([query stage-number boolean-expression]
+ ([query :- :metabase.lib.schema/query
+ stage-number :- [:maybe :int]
+ boolean-expression]
(let [stage-number (clojure.core/or stage-number -1)
- new-filter (->filter-clause query stage-number boolean-expression)]
+ new-filter (lib.common/->op-arg query stage-number boolean-expression)]
(lib.util/update-query-stage query stage-number assoc :filter new-filter))))
(defn- and-clause? [clause]
(clojure.core/and (vector? clause)
(clojure.core/= (first clause) :and)))
-(mu/defn current-filter :- [:maybe ::schema.expression/boolean]
+(mu/defn current-filter :- [:maybe ::schema.common/external-op]
"Returns the current filter in stage with `stage-number` of `query`.
If `stage-number` is omitted, the last stage is used.
See also [[metabase.lib.util/query-stage]]."
- ([query :- :metabase.lib.schema/query] (current-filter query -1))
+ ([query :- :metabase.lib.schema/query] (current-filter query nil))
([query :- :metabase.lib.schema/query
- stage-number :- :int]
- (:filter (lib.util/query-stage query stage-number))))
+ stage-number :- [:maybe :int]]
+ (-> (lib.util/query-stage query (clojure.core/or stage-number -1))
+ :filter
+ lib.common/external-op)))
-(mu/defn current-filters :- [:sequential ::schema.expression/boolean]
+(mu/defn current-filters :- [:sequential ::schema.common/external-op]
"Returns the current filters in stage with `stage-number` of `query`.
If `stage-number` is omitted, the last stage is used. Logicaly, the
filter attached to the query is the conjunction of the expressions
in the returned list. If the returned list is empty, then there is no
filter attached to the query.
See also [[metabase.lib.util/query-stage]]."
- ([query :- :metabase.lib.schema/query] (current-filters query -1))
+ ([query :- :metabase.lib.schema/query] (current-filters query nil))
([query :- :metabase.lib.schema/query
- stage-number :- :int]
- (if-let [existing-filter (:filter (lib.util/query-stage query stage-number))]
+ stage-number :- [:maybe :int]]
+ (if-let [existing-filter (:filter (lib.util/query-stage query (clojure.core/or stage-number -1)))]
(if (and-clause? existing-filter)
- (subvec existing-filter 2)
- [existing-filter])
+ (mapv lib.common/external-op (subvec existing-filter 2))
+ [(lib.common/external-op existing-filter)])
[])))
(defn- conjoin [existing-filter new-filter]
@@ -304,23 +244,28 @@
yet, builds a conjunction with the current filter otherwise."
([query :- :metabase.lib.schema/query
boolean-expression]
- (metabase.lib.filter/add-filter query -1 boolean-expression))
+ (metabase.lib.filter/add-filter query nil boolean-expression))
([query :- :metabase.lib.schema/query
- stage-number :- :int
+ stage-number :- [:maybe :int]
boolean-expression]
(let [stage-number (clojure.core/or stage-number -1)
- new-filter (->filter-clause query stage-number boolean-expression)]
+ new-filter (lib.common/->op-arg query stage-number boolean-expression)]
(lib.util/update-query-stage query stage-number update :filter conjoin new-filter))))
(mu/defn replace-filter :- :metabase.lib.schema/query
"Replaces the expression with `target-uuid` with `boolean-expression` the filter of `query`."
- ([query target-uuid boolean-expression]
- (metabase.lib.filter/replace-filter query -1 target-uuid boolean-expression))
+ ([query :- :metabase.lib.schema/query
+ target-uuid :- :string
+ boolean-expression]
+ (metabase.lib.filter/replace-filter query nil target-uuid boolean-expression))
- ([query stage-number target-uuid boolean-expression]
+ ([query :- :metabase.lib.schema/query
+ stage-number :- [:maybe :int]
+ target-uuid :- :string
+ boolean-expression]
(let [stage-number (clojure.core/or stage-number -1)
- new-filter (->filter-clause query stage-number boolean-expression)]
+ new-filter (lib.common/->op-arg query stage-number boolean-expression)]
(lib.util/update-query-stage query stage-number
update :filter
lib.util/replace-clause target-uuid new-filter))))
diff --git a/src/metabase/lib/join.cljc b/src/metabase/lib/join.cljc
index 2d1476a61a9..ed067364cc8 100644
--- a/src/metabase/lib/join.cljc
+++ b/src/metabase/lib/join.cljc
@@ -97,6 +97,9 @@
[query stage-number f]
(->join-clause query stage-number (f query stage-number)))
+;; TODO this is basically the same as lib.common/->op-args,
+;; but requiring lib.common leads to crircular dependencies:
+;; join -> common -> field -> join.
(defmulti ^:private ->join-condition
{:arglists '([query stage-number x])}
(fn [_query _stage-number x]
@@ -106,6 +109,11 @@
[_query _stage-number x]
x)
+(defmethod ->join-condition :lib/external-op
+ [query stage-number {:keys [operator options args] :or {options {}}}]
+ (->join-condition query stage-number
+ (lib.options/ensure-uuid (into [operator options] args))))
+
(defmethod ->join-condition :dispatch-type/fn
[query stage-number f]
(->join-condition query stage-number (f query stage-number)))
diff --git a/src/metabase/lib/schema/common.cljc b/src/metabase/lib/schema/common.cljc
index fc041acf381..6bf0afd9878 100644
--- a/src/metabase/lib/schema/common.cljc
+++ b/src/metabase/lib/schema/common.cljc
@@ -31,3 +31,9 @@
[:fn
{:error/message "valid base type"}
#(isa? % :type/*)])
+
+(mr/def ::external-op
+ [:map
+ [:operator [:or :string :keyword]]
+ [:options {:optional true} ::options]
+ [:args [:sequential :any]]])
diff --git a/test/metabase/lib/aggregation_test.cljc b/test/metabase/lib/aggregation_test.cljc
index 75c120b7711..61efb32b310 100644
--- a/test/metabase/lib/aggregation_test.cljc
+++ b/test/metabase/lib/aggregation_test.cljc
@@ -3,6 +3,7 @@
[clojure.test :refer [are deftest is testing]]
[metabase.lib.convert :as lib.convert]
[metabase.lib.core :as lib]
+ [metabase.lib.field :as lib.field]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
[metabase.lib.query :as lib.query]
@@ -15,8 +16,7 @@
(defn- is-fn? [op tag args expected-args]
(let [f (apply op args)]
(is (fn? f))
- (is (=? (into [tag {:lib/uuid string?}]
- expected-args)
+ (is (=? {:operator tag, :args expected-args}
(f {:lib/metadata meta/metadata} -1)))))
(deftest ^:parallel aggregation-test
@@ -24,9 +24,8 @@
venues-category-id-metadata (lib.metadata/field q1 nil "VENUES" "CATEGORY_ID")
venue-field-check [:field {:base-type :type/Integer, :lib/uuid string?} (meta/id :venues :category-id)]]
(testing "count"
- (testing "without query/stage-number, return a function for later resolution"
- (is-fn? lib/count :count [] [])
- (is-fn? lib/count :count [venues-category-id-metadata] [venue-field-check])))
+ (is-fn? lib/count :count [] [])
+ (is-fn? lib/count :count [venues-category-id-metadata] [venue-field-check]))
(testing "single arg aggregations"
(doseq [[op tag] [[lib/avg :avg]
[lib/max :max]
@@ -35,8 +34,7 @@
[lib/sum :sum]
[lib/stddev :stddev]
[lib/distinct :distinct]]]
- (testing "without query/stage-number, return a function for later resolution"
- (is-fn? op tag [venues-category-id-metadata] [venue-field-check]))))))
+ (is-fn? op tag [venues-category-id-metadata] [venue-field-check])))))
(defn- aggregation-display-name [aggregation-clause]
(lib.metadata.calculation/display-name lib.tu/venues-query -1 aggregation-clause))
@@ -173,14 +171,27 @@
[:expression {:base-type :type/Integer, :lib/uuid (str (random-uuid))} "double-price"]])))))
(deftest ^:parallel aggregate-test
- (is (=? {:lib/type :mbql/query,
- :database (meta/id) ,
- :type :pipeline,
- :stages [{:lib/type :mbql.stage/mbql,
- :source-table (meta/id :venues) ,
- :lib/options {:lib/uuid string?},
- :aggregation [[:sum {:lib/uuid string?}
- [:field {:base-type :type/Integer, :lib/uuid string?} (meta/id :venues :category-id)]]]}]}
- (-> (lib/query-for-table-name meta/metadata-provider "VENUES")
- (lib/aggregate (lib/sum (lib/field "VENUES" "CATEGORY_ID")))
- (dissoc :lib/metadata)))))
+ (let [q (lib/query-for-table-name meta/metadata-provider "VENUES")
+ result-query
+ {:lib/type :mbql/query,
+ :database (meta/id) ,
+ :type :pipeline,
+ :stages [{:lib/type :mbql.stage/mbql,
+ :source-table (meta/id :venues) ,
+ :lib/options {:lib/uuid string?},
+ :aggregation [[:sum {:lib/uuid string?}
+ [:field
+ {:base-type :type/Integer, :lib/uuid string?}
+ (meta/id :venues :category-id)]]]}]}]
+
+ (testing "with helper function"
+ (is (=? result-query
+ (-> q
+ (lib/aggregate (lib/sum (lib/field "VENUES" "CATEGORY_ID")))
+ (dissoc :lib/metadata)))))
+ (testing "with external format"
+ (is (=? result-query
+ (-> q
+ (lib/aggregate {:operator :sum
+ :args [(lib.field/field q (lib.metadata/field q nil "VENUES" "CATEGORY_ID"))]})
+ (dissoc :lib/metadata)))))))
diff --git a/test/metabase/lib/expression_test.cljc b/test/metabase/lib/expression_test.cljc
index 1f32dd4a46d..17fb916f3c6 100644
--- a/test/metabase/lib/expression_test.cljc
+++ b/test/metabase/lib/expression_test.cljc
@@ -7,6 +7,7 @@
[metabase.lib.expression :as lib.expression]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
[metabase.lib.schema :as lib.schema]
+ [metabase.lib.schema.common :as schema.common]
[metabase.lib.schema.expression :as schema.expression]
[metabase.lib.test-metadata :as meta]
[metabase.lib.test-util :as lib.tu]))
@@ -79,9 +80,11 @@
(lib/lower string-field) :type/Text])]
(testing (str "expression: " (pr-str expr))
(let [query (-> (lib/query-for-table-name meta/metadata-provider "VENUES")
- (lib/expression "myexpr" expr))]
+ (lib/expression "myexpr" expr))
+ resolved (lib.expression/resolve-expression query 0 "myexpr")]
(is (mc/validate ::lib.schema/query query))
- (is (= typ (schema.expression/type-of (lib.expression/resolve-expression query 0 "myexpr")))))))))
+ (is (mc/validate ::schema.common/external-op resolved))
+ (is (= typ (schema.expression/type-of resolved))))))))
(deftest ^:parallel col-info-expression-ref-test
(is (=? {:base_type :type/Integer
diff --git a/test/metabase/lib/filter_test.cljc b/test/metabase/lib/filter_test.cljc
index 08fb8925b85..69d3c1e98b4 100644
--- a/test/metabase/lib/filter_test.cljc
+++ b/test/metabase/lib/filter_test.cljc
@@ -2,17 +2,14 @@
(:require
[clojure.test :refer [deftest is testing]]
[metabase.lib.core :as lib]
+ [metabase.lib.field :as lib.field]
[metabase.lib.metadata :as lib.metadata]
- [metabase.lib.options :as lib.options]
[metabase.lib.test-metadata :as meta]
#?@(:cljs ([metabase.test-runner.assert-exprs.approximately-equal]))))
-(defn- test-clause [result-filter f ->f & args]
- (testing "with query/stage-number, return clause right away"
- (is (=? result-filter
- (apply f {:lib/metadata meta/metadata} -1 args))))
- (testing "without query/stage-number, return a function for later resolution"
- (let [f' (apply ->f args)]
+(defn- test-clause [result-filter f & args]
+ (testing "return a function for later resolution"
+ (let [f' (apply f args)]
(is (fn? f'))
(is (=? result-filter
(f' {:lib/metadata meta/metadata} -1))))))
@@ -28,79 +25,73 @@
categories-id-metadata (lib.metadata/stage-column q2 -1 "ID")
checkins-date-metadata (lib.metadata/field q3 nil "CHECKINS" "DATE")]
(testing "comparisons"
- (doseq [[op f ->f] [[:= lib/= lib/->=]
- [:!= lib/!= lib/->!=]
- [:< lib/< lib/-><]
- [:<= lib/<= lib/-><=]
- [:> lib/> lib/->>]
- [:>= lib/>= lib/->>=]]]
+ (doseq [[op f] [[:= lib/=]
+ [:!= lib/!=]
+ [:< lib/<]
+ [:<= lib/<=]
+ [:> lib/>]
+ [:>= lib/>=]]]
(test-clause
- [op
- {:lib/uuid string?}
- [:field {:lib/uuid string?} (meta/id :venues :category-id)]
- [:field {:base-type :type/BigInteger, :lib/uuid string?} "ID"]]
- f ->f
+ {:operator op
+ :args [[:field {:lib/uuid string?} (meta/id :venues :category-id)]
+ [:field {:base-type :type/BigInteger, :lib/uuid string?} "ID"]]}
+ f
venues-category-id-metadata
categories-id-metadata)))
(testing "between"
(test-clause
- [:between
- {:lib/uuid string?}
- [:field {:lib/uuid string?} (meta/id :venues :category-id)]
- 42
- [:field {:base-type :type/BigInteger, :lib/uuid string?} "ID"]]
- lib/between lib/->between
+ {:operator :between
+ :args [[:field {:lib/uuid string?} (meta/id :venues :category-id)]
+ 42
+ [:field {:base-type :type/BigInteger, :lib/uuid string?} "ID"]]}
+ lib/between
venues-category-id-metadata
42
categories-id-metadata))
(testing "inside"
(test-clause
- [:inside
- {:lib/uuid string?}
- [:field {:base-type :type/Float, :lib/uuid string?} (meta/id :venues :latitude)]
- [:field {:base-type :type/Float, :lib/uuid string?} (meta/id :venues :longitude)]
- 42.7 13 4 27.3]
- lib/inside lib/->inside
+ {:operator :inside
+ :args [[:field {:base-type :type/Float, :lib/uuid string?} (meta/id :venues :latitude)]
+ [:field {:base-type :type/Float, :lib/uuid string?} (meta/id :venues :longitude)]
+ 42.7 13 4 27.3]}
+ lib/inside
venues-latitude-metadata
venues-longitude-metadata
42.7 13 4 27.3))
(testing "emptiness"
- (doseq [[op f ->f] [[:is-null lib/is-null lib/->is-null]
- [:not-null lib/not-null lib/->not-null]
- [:is-empty lib/is-empty lib/->is-empty]
- [:not-empty lib/not-empty lib/->not-empty]]]
+ (doseq [[op f] [[:is-null lib/is-null]
+ [:not-null lib/not-null]
+ [:is-empty lib/is-empty]
+ [:not-empty lib/not-empty]]]
(test-clause
- [op
- {:lib/uuid string?}
- [:field {:lib/uuid string?} (meta/id :venues :name)]]
- f ->f
+ {:operator op
+ :args [[:field {:lib/uuid string?} (meta/id :venues :name)]]}
+ f
venues-name-metadata)))
(testing "string tests"
- (doseq [[op f ->f] [[:starts-with lib/starts-with lib/->starts-with]
- [:ends-with lib/ends-with lib/->ends-with]
- [:contains lib/contains lib/->contains]
- [:does-not-contain lib/does-not-contain lib/->does-not-contain]]]
+ (doseq [[op f] [[:starts-with lib/starts-with]
+ [:ends-with lib/ends-with]
+ [:contains lib/contains]
+ [:does-not-contain lib/does-not-contain]]]
(test-clause
- [op
- {:lib/uuid string?}
- [:field {:lib/uuid string?} (meta/id :venues :name)]
- "part"]
- f ->f
+ {:operator op
+ :args [[:field {:lib/uuid string?} (meta/id :venues :name)]
+ "part"]}
+ f
venues-name-metadata
"part")))
(testing "time-interval"
(test-clause
- [:time-interval
- {:lib/uuid string?}
- [:field {:base-type :type/Date, :lib/uuid string?} (meta/id :checkins :date)]
- 3
- :day]
- lib/time-interval lib/->time-interval
+ {:operator :time-interval
+ :args [[:field {:base-type :type/Date, :lib/uuid string?} (meta/id :checkins :date)]
+ 3
+ :day]}
+ lib/time-interval
checkins-date-metadata
3
:day))
@@ -108,16 +99,15 @@
(testing "segment"
(doseq [id [7 "6"]]
(test-clause
- [:segment {:lib/uuid string?} id]
- lib/segment lib/->segment
+ {:operator :segment
+ :args [id]}
+ lib/segment
id)))))
(deftest ^:parallel filter-test
(let [q1 (lib/query-for-table-name meta/metadata-provider "CATEGORIES")
q2 (lib/saved-question-query meta/metadata-provider meta/saved-question)
venues-category-id-metadata (lib.metadata/field q1 nil "VENUES" "CATEGORY_ID")
- venues-name-metadata (lib.metadata/field q1 nil "VENUES" "NAME")
- categories-id-metadata (lib.metadata/stage-column q2 -1 "ID")
original-filter
[:between
{:lib/uuid string?}
@@ -135,62 +125,26 @@
(testing "no filter"
(is (nil? (lib/current-filter q1)))
(is (= [] (lib/current-filters q2))))
- (testing "setting a simple filter"
- (let [result-query
- (lib/filter q1 (lib/between {:lib/metadata meta/metadata} -1 venues-category-id-metadata 42 100))]
- (is (=? simple-filtered-query
- (dissoc result-query :lib/metadata)))
- (testing "and getting the current filter"
- (is (=? original-filter
- (lib/current-filter result-query)))
- (is (=? [original-filter]
- (lib/current-filters result-query))))))
- (testing "setting a simple filter thunk"
- (is (=? simple-filtered-query
- (-> q1
- (lib/filter (lib/->between venues-category-id-metadata 42 100))
- (dissoc :lib/metadata)))))
+ (testing "setting a simple filter via the helper function"
+ (let [result-query
+ (lib/filter q1 (lib/between venues-category-id-metadata 42 100))
+ result-filter {:operator (-> original-filter first name)
+ :options (second original-filter)
+ :args (subvec original-filter 2)}]
+ (is (=? simple-filtered-query
+ (dissoc result-query :lib/metadata)))
+ (testing "and getting the current filter"
+ (is (=? result-filter
+ (lib/current-filter result-query)))
+ (is (=? [result-filter]
+ (lib/current-filters result-query))))))
(testing "setting a simple filter expression"
(is (=? simple-filtered-query
(-> q1
- (lib/filter [:between venues-category-id-metadata 42 100])
- (dissoc :lib/metadata)))))
-
- (testing "setting a nested filter expression"
- (is (=? {:lib/type :mbql/query,
- :database (meta/id),
- :type :pipeline,
- :stages
- [{:lib/type :mbql.stage/mbql,
- :source-table (meta/id :categories)
- :lib/options #:lib{:uuid string?}
- :filter
- [:or
- #:lib{:uuid string?}
- [:between
- #:lib{:uuid string?}
- [:field {:base-type :type/Integer, :lib/uuid string?} (meta/id :venues :category-id)]
- 42
- 100]
- [:and
- #:lib{:uuid string?}
- [:=
- #:lib{:uuid string?}
- [:field {:base-type :type/Integer, :lib/uuid string?} (meta/id :venues :category-id)]
- 242
- [:field {:base-type :type/BigInteger, :lib/uuid string?} "ID"]]
- [:contains
- #:lib{:uuid string?}
- [:field {:base-type :type/Text, :lib/uuid string?} (meta/id :venues :name)]
- "part"]]]}]}
- (-> q1
- (lib/filter [:or
- [:between venues-category-id-metadata 42 100]
- [:and
- [:= venues-category-id-metadata 242 categories-id-metadata]
- [:contains venues-name-metadata "part"]]])
+ (lib/filter {:operator :between
+ :args [(lib.field/field q1 venues-category-id-metadata) 42 100]})
(dissoc :lib/metadata)))))))
(deftest ^:parallel add-filter-test
@@ -204,41 +158,46 @@
(meta/id :venues :category-id)]
42
100]
+ first-result-filter
+ {:operator (-> first-filter first name)
+ :options (second first-filter)
+ :args (subvec first-filter 2)}
second-filter
[:starts-with
{:lib/uuid string?}
[:field {:base-type :type/Text, :lib/uuid string?} (meta/id :venues :name)]
"prefix"]
+ second-result-filter
+ {:operator (-> second-filter first name)
+ :options (second second-filter)
+ :args (subvec second-filter 2)}
third-filter
[:contains
{:lib/uuid string?}
[:field {:base-type :type/Text, :lib/uuid string?} (meta/id :venues :name)]
"part"]
+ third-result-filter
+ {:operator (-> third-filter first name)
+ :options (second third-filter)
+ :args (subvec third-filter 2)}
first-add
(lib/add-filter simple-query
- (lib/->between
- (lib.metadata/field simple-query nil "VENUES" "CATEGORY_ID")
+ (lib/between
+ (lib/field "VENUES" "CATEGORY_ID")
42
100))
filtered-query
(assoc-in simple-query [:stages 0 :filter] first-filter)
second-add
- (lib/add-filter first-add
- (lib/starts-with
- {:lib/metadata meta/metadata}
- 0
- venues-name-metadata
- "prefix"))
+ (lib/add-filter first-add {:operator "starts-with"
+ :args [(lib.field/field simple-query venues-name-metadata) "prefix"]})
and-query
(assoc-in filtered-query
[:stages 0 :filter]
- [:and
- {:lib/uuid string?}
- first-filter
- second-filter])
+ [:and {:lib/uuid string?} first-filter second-filter])
third-add
- (lib/add-filter second-add
- (lib/->contains venues-name-metadata "part"))
+ (lib/add-filter second-add {:operator :contains
+ :args [(lib.field/field simple-query venues-name-metadata) "part"]})
extended-and-query
(assoc-in filtered-query
[:stages 0 :filter]
@@ -252,15 +211,15 @@
"part"]])]
(testing "adding an initial filter"
(is (=? filtered-query first-add))
- (is (=? [first-filter]
+ (is (=? [first-result-filter]
(lib/current-filters first-add))))
(testing "conjoining to filter"
(is (=? and-query second-add))
- (is (=? [first-filter second-filter]
+ (is (=? [first-result-filter second-result-filter]
(lib/current-filters second-add))))
(testing "conjoining to conjunction filter"
(is (=? extended-and-query third-add))
- (is (=? [first-filter second-filter third-filter]
+ (is (=? [first-result-filter second-result-filter third-result-filter]
(lib/current-filters third-add))))))
(deftest ^:parallel replace-filter-test
@@ -269,9 +228,9 @@
venues-category-id-metadata (lib.metadata/field q1 nil "VENUES" "CATEGORY_ID")
simple-filtered-query
(-> q1
- (lib/filter (lib/->between venues-category-id-metadata 42 100)))
+ (lib/filter (lib/between venues-category-id-metadata 42 100)))
between-uuid (-> (lib/current-filter simple-filtered-query)
- lib.options/options
+ :options
:lib/uuid)
result-query
(assoc-in simple-filtered-query
@@ -286,57 +245,16 @@
(is (=? result-query
(lib/replace-filter simple-filtered-query
between-uuid
- (lib/starts-with
- {:lib/metadata meta/metadata}
- 0
- venues-name-metadata
- "part")))))
+ (lib/starts-with venues-name-metadata "part")))))
(testing "setting a simple filter thunk"
(is (=? result-query
(lib/replace-filter simple-filtered-query
between-uuid
- (lib/->starts-with venues-name-metadata "part")))))
+ {:operator "starts-with"
+ :args [(lib.field/field q1 venues-name-metadata) "part"]}))))
(testing "setting a simple filter expression"
(is (=? result-query
(lib/replace-filter simple-filtered-query
between-uuid
- [:starts-with venues-name-metadata "part"]))))
-
- (let [contains-uuid (str (random-uuid))
- nested-filtered-query
- {:lib/type :mbql/query,
- :database (meta/id),
- :type :pipeline,
- :stages
- [{:lib/type :mbql.stage/mbql,
- :source-table (meta/id :categories)
- :lib/options {:lib/uuid (str (random-uuid))}
- :filter
- [:or
- {:lib/uuid (str (random-uuid))}
- [:between
- {:lib/uuid (str (random-uuid))}
- [:field {:base-type :type/Integer, :lib/uuid (str (random-uuid))} (meta/id :venues :category-id)]
- 42
- 100]
- [:and
- {:lib/uuid (str (random-uuid))}
- [:contains
- {:lib/uuid contains-uuid}
- [:field {:base-type :type/Text, :lib/uuid (str (random-uuid))} (meta/id :venues :name)]
- "part"]
- [:=
- {:lib/uuid (str (random-uuid))}
- [:field {:base-type :type/Integer, :lib/uuid (str (random-uuid))} (meta/id :venues :category-id)]
- 242
- [:field {:base-type :type/BigInteger, :lib/uuid (str (random-uuid))} "ID"]]]]}]}]
- (testing "setting a nested filter expression"
- (is (=? (assoc-in nested-filtered-query
- [:stages 0 :filter 3 2]
- [:starts-with
- {:lib/uuid string?}
- [:field {:base-type :type/Text, :lib/uuid string?} (meta/id :venues :name)]
- "part"])
- (lib/replace-filter nested-filtered-query
- contains-uuid
- [:starts-with venues-name-metadata "part"])))))))
+ {:operator :starts-with
+ :args [(lib.field/field q1 venues-name-metadata) "part"]}))))))
diff --git a/test/metabase/lib/join_test.cljc b/test/metabase/lib/join_test.cljc
index 793567922a2..c88c71f16c6 100644
--- a/test/metabase/lib/join_test.cljc
+++ b/test/metabase/lib/join_test.cljc
@@ -2,6 +2,7 @@
(:require
[clojure.test :refer [deftest is testing]]
[metabase.lib.core :as lib]
+ [metabase.lib.field :as lib.field]
[metabase.lib.join :as lib.join]
[metabase.lib.metadata :as lib.metadata]
[metabase.lib.metadata.calculation :as lib.metadata.calculation]
@@ -15,7 +16,7 @@
(let [query (lib/query meta/metadata-provider (meta/table-metadata :venues))
join-clause (-> ((lib/join-clause
(meta/table-metadata :categories)
- (lib/->=
+ (lib/=
(lib/field (meta/id :venues :category-id))
(lib/with-join-alias (lib/field (meta/id :categories :id)) "CATEGORIES__via__CATEGORY_ID")))
query -1)
@@ -41,11 +42,13 @@
{:lib/uuid string?}
[:field {:lib/uuid string?} (meta/id :venues :category-id)]
[:field {:lib/uuid string?} (meta/id :categories :id)]]}]}]}
- (-> (lib/query-for-table-name meta/metadata-provider "VENUES")
- (lib/join (lib/query-for-table-name meta/metadata-provider "CATEGORIES")
- (lib/->= (lib/field "VENUES" "CATEGORY_ID")
- (lib/field "CATEGORIES" "ID")))
- (dissoc :lib/metadata)))))
+ (let [q (lib/query-for-table-name meta/metadata-provider "VENUES")]
+ (-> q
+ (lib/join (lib/query-for-table-name meta/metadata-provider "CATEGORIES")
+ {:operator :=
+ :args [(lib.field/field q (lib.metadata/field q nil "VENUES" "CATEGORY_ID"))
+ (lib.field/field q (lib.metadata/field q nil "CATEGORIES" "ID"))]})
+ (dissoc :lib/metadata))))))
(deftest ^:parallel join-saved-question-test
(is (=? {:lib/type :mbql/query
@@ -65,8 +68,8 @@
[:field {:lib/uuid string?} (meta/id :categories :id)]]}]}]}
(-> (lib/query-for-table-name meta/metadata-provider "CATEGORIES")
(lib/join (lib/saved-question-query meta/metadata-provider meta/saved-question)
- (lib/->= (lib/field "VENUES" "CATEGORY_ID")
- (lib/field "CATEGORIES" "ID")))
+ (lib/= (lib/field "VENUES" "CATEGORY_ID")
+ (lib/field "CATEGORIES" "ID")))
(dissoc :lib/metadata)))))
(deftest ^:parallel join-condition-field-metadata-test
@@ -76,7 +79,7 @@
venues-category-id-metadata (lib.metadata/field q1 nil "VENUES" "CATEGORY_ID")
categories-id-metadata (lib.metadata/stage-column q2 "ID")]
(testing "lib/join-clause: return a function that can be resolved later"
- (let [f (lib/join-clause q2 (lib/->= venues-category-id-metadata categories-id-metadata))]
+ (let [f (lib/join-clause q2 (lib/= venues-category-id-metadata categories-id-metadata))]
(is (fn? f))
(is (=? {:lib/type :mbql/join
:lib/options {:lib/uuid string?}
@@ -98,7 +101,7 @@
[:field {:lib/uuid string?} (meta/id :venues :category-id)]
[:field {:base-type :type/BigInteger, :lib/uuid string?} "ID"]]}]}]}
(-> q1
- (lib/join q2 (lib/->= venues-category-id-metadata categories-id-metadata))
+ (lib/join q2 (lib/= venues-category-id-metadata categories-id-metadata))
(dissoc :lib/metadata)))))))
;;; FIXME
diff --git a/test/metabase/lib/query_test.cljc b/test/metabase/lib/query_test.cljc
index f2d430a3574..642940c94b4 100644
--- a/test/metabase/lib/query_test.cljc
+++ b/test/metabase/lib/query_test.cljc
@@ -15,7 +15,7 @@
(lib/aggregate (lib/sum (lib/field (meta/id :venues :price)))))
;; wrong arity: there's a bug in our Kondo config, see https://metaboat.slack.com/archives/C04DN5VRQM6/p1679022185079739?thread_ts=1679022025.317059&cid=C04DN5VRQM6
query (-> #_{:clj-kondo/ignore [:invalid-arity]}
- (lib/filter query (lib/= query -1 (lib/field (meta/id :venues :name)) "Toucannery"))
+ (lib/filter query (lib/= (lib/field (meta/id :venues :name)) "Toucannery"))
(lib/breakout (lib/field (meta/id :venues :category-id)))
(lib/order-by (lib/field (meta/id :venues :id)))
(lib/limit 100))]
diff --git a/test/metabase/lib/util_test.cljc b/test/metabase/lib/util_test.cljc
index 6ef3efb1c45..aeb3da851b7 100644
--- a/test/metabase/lib/util_test.cljc
+++ b/test/metabase/lib/util_test.cljc
@@ -220,7 +220,7 @@
(deftest ^:parallel replace-clause-test
(checking "can be called with anything"
- [x gen/any]
+ [x gen/any-equatable]
(is (identical? x (lib.util/replace-clause x
(str (random-uuid))
(random-uuid)))))
--
GitLab