Skip to content

Commit

Permalink
[QP, lib] Allow multiple arguments to :contains, :starts-with, etc.
Browse files Browse the repository at this point in the history
These string matching clauses only allowed two arguments previously.
Typically `[:contains field x]` to match a field against a literal.

This adds similar desugaring for `:contains`, `:does-not-contain`,
`:starts-with` and `:ends-with` that is currently done for
multi-argument `:=` and `:!=`:

```clojure
[:contains field x y z] ;; ->
[:or [:contains field x] [:contains field y] [:contains field z]]

[:does-not-contain field x y z] ;; ->
[:and [:does-not-contain field x]
      [:does-not-contain field y]
      [:does-not-contain field z]]
```
  • Loading branch information
bshepherdson committed Apr 29, 2024
1 parent 40b17c9 commit f17148b
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 27 deletions.
23 changes: 19 additions & 4 deletions src/metabase/legacy_mbql/schema.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -784,13 +784,28 @@
;; default true
[:case-sensitive {:optional true} :boolean]])

(defclause starts-with, field StringExpressionArg, string-or-field StringExpressionArg, options (optional StringFilterOptions))
(defclause ends-with, field StringExpressionArg, string-or-field StringExpressionArg, options (optional StringFilterOptions))
(defclause contains, field StringExpressionArg, string-or-field StringExpressionArg, options (optional StringFilterOptions))
(defclause starts-with
field StringExpressionArg
string-or-field StringExpressionArg
more-values-or-fields (rest StringExpressionArg)
options (optional StringFilterOptions))
(defclause ends-with
field StringExpressionArg
string-or-field StringExpressionArg
more-values-or-fields (rest StringExpressionArg)
options (optional StringFilterOptions))
(defclause contains
field StringExpressionArg
string-or-field StringExpressionArg
more-values-or-fields (rest StringExpressionArg)
options (optional StringFilterOptions))

;; SUGAR: this is rewritten as [:not [:contains ...]]
(defclause ^:sugar does-not-contain
field StringExpressionArg, string-or-field StringExpressionArg, options (optional StringFilterOptions))
field StringExpressionArg
string-or-field StringExpressionArg
more-values-or-fields (rest StringExpressionArg)
options (optional StringFilterOptions))

(def ^:private TimeIntervalOptions
;; Should we include partial results for the current day/month/etc? Defaults to `false`; set this to `true` to
Expand Down
33 changes: 26 additions & 7 deletions src/metabase/legacy_mbql/util.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -237,17 +237,23 @@
[:relative-datetime n unit]]))

(defn desugar-does-not-contain
"Rewrite `:does-not-contain` filter clauses as simpler `:not` clauses."
"Rewrite `:does-not-contain` filter clauses as simpler `[:not [:contains ...]]` clauses.
Note that [[desugar-multi-argument-comparisons]] will have already desugared any 3+ argument `:does-not-contain` to
several `[:and [:does-not-contain ...] [:does-not-contain ...] ...]` clauses, which then get rewritten here into
`[:and [:not [:contains ...]] [:not [:contains ...]]]`."
[m]
(lib.util.match/replace m
[:does-not-contain & args]
[:not (into [:contains] args)]))

(defn desugar-equals-and-not-equals-with-extra-args
"`:=` and `!=` clauses with more than 2 args automatically get rewritten as compound filters.
(defn desugar-multi-argument-comparisons
"`:=`, `!=`, `:contains`, `:does-not-contain`, `:starts-with` and `:ends-with` clauses with more than 2 args
automatically get rewritten as compound filters.
[:= field x y] -> [:or [:= field x] [:= field y]]
[:!= field x y] -> [:and [:!= field x] [:!= field y]]"
[:= field x y] -> [:or [:= field x] [:= field y]]
[:!= field x y] -> [:and [:!= field x] [:!= field y]]
[:does-not-contain field x y] -> [:and [:does-not-contain field x] [:does-not-contain field y]]"
[m]
(lib.util.match/replace m
[:= field x y & more]
Expand All @@ -256,7 +262,20 @@

[:!= field x y & more]
(apply vector :and (for [x (concat [x y] more)]
[:!= field x]))))
[:!= field x]))

;; The last argument in `more` might be an options map; preserve it if so.
;; If `y` is that map, this just fails to match. (That's a single argument plus options, so no need to desugar.)
[(op :guard #{:contains :does-not-contain :starts-with :ends-with})
field x (y :guard (complement map?)) & more]
(let [end (last more)
[more tail] (if (map? end)
[(butlast more) [end]]
[more nil])]
(apply vector
(if (= op :does-not-contain) :and :or)
(for [x (concat [x y] more)]
(into [op field x] tail))))))

(defn desugar-current-relative-datetime
"Replace `relative-datetime` clauses like `[:relative-datetime :current]` with `[:relative-datetime 0 <unit>]`.
Expand Down Expand Up @@ -408,7 +427,7 @@
[filter-clause :- mbql.s/Filter]
(-> filter-clause
desugar-current-relative-datetime
desugar-equals-and-not-equals-with-extra-args
desugar-multi-argument-comparisons
desugar-does-not-contain
desugar-time-interval
desugar-is-null-and-not-null
Expand Down
17 changes: 8 additions & 9 deletions src/metabase/lib/schema/filter.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -79,18 +79,17 @@
(def ^:private string-filter-options
[:map [:case-sensitive {:optional true} :boolean]]) ; default true

;; binary [:ref ::expression/string] filter clauses. These also accept a `:case-sensitive` option
;; N-ary [:ref ::expression/string] filter clauses. These also accept a `:case-sensitive` option.
;; Requires at least 2 string-shaped args. If there are more than 2, `[:contains x a b]` is equivalent to
;; `[:or [:contains x a] [:contains x b]]`.
;;
;; `:does-not-contain` is sugar for `[:not [:contains ...]]`:
;;
;; [:does-not-contain ...] = [:not [:contains ...]]
;; `[:does-not-contain ...]` = `[:not [:contains ...]]`
(doseq [op [:starts-with :ends-with :contains :does-not-contain]]
(mbql-clause/define-mbql-clause op :- :type/Boolean
[:tuple
[:= {:decode/normalize common/normalize-keyword} op]
[:merge ::common/options string-filter-options]
#_whole [:ref ::expression/string]
#_part [:ref ::expression/string]]))
[:schema [:catn {:error/message (str "Valid " op " clause")}
[:tag [:= {:decode/normalize common/normalize-keyword} op]]
[:options [:merge ::common/options string-filter-options]]
[:args [:repeat {:min 2} [:schema [:ref ::expression/string]]]]]]))

(def ^:private time-interval-options
[:map [:include-current {:optional true} :boolean]]) ; default false
Expand Down
3 changes: 2 additions & 1 deletion src/metabase/lib/schema/mbql_clause.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@
return-type)
nil))

;;; TODO -- add more stuff.
;;; TODO: Support options more nicely - these don't allow for overriding the options, but we have a few cases where that
;;; is necessary. See for example the inclusion of `string-filter-options` in [[metabase.lib.filter]].

(defn catn-clause-schema
"Helper intended for use with [[define-mbql-clause]]. Create an MBQL clause schema with `:catn`. Use this for clauses
Expand Down
36 changes: 30 additions & 6 deletions test/metabase/legacy_mbql/util_test.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -374,12 +374,36 @@
(mbql.u/desugar-filter-clause [:not-empty [:field 1 {:base-type :type/DateTime}]])))))

(t/deftest ^:parallel desugar-does-not-contain-test
(t/testing "desugaring does-not-contain without options"
(t/is (= [:not [:contains [:field 1 nil] "ABC"]]
(mbql.u/desugar-filter-clause [:does-not-contain [:field 1 nil] "ABC"]))))
(t/testing "desugaring does-not-contain *with* options"
(t/is (= [:not [:contains [:field 1 nil] "ABC" {:case-sensitive false}]]
(mbql.u/desugar-filter-clause [:does-not-contain [:field 1 nil] "ABC" {:case-sensitive false}])))))
(t/testing "desugaring does-not-contain"
(t/testing "without options"
(t/is (= [:not [:contains [:field 1 nil] "ABC"]]
(mbql.u/desugar-filter-clause [:does-not-contain [:field 1 nil] "ABC"]))))
(t/testing "*with* options"
(t/is (= [:not [:contains [:field 1 nil] "ABC" {:case-sensitive false}]]
(mbql.u/desugar-filter-clause [:does-not-contain [:field 1 nil] "ABC" {:case-sensitive false}]))))
(t/testing "desugaring does-not-contain with multiple arguments"
(t/testing "without options"
(t/is (= [:and
[:not [:contains [:field 1 nil] "ABC"]]
[:not [:contains [:field 1 nil] "XYZ"]]]
(mbql.u/desugar-filter-clause [:does-not-contain [:field 1 nil] "ABC" "XYZ"])))
(t/is (= [:and
[:not [:contains [:field 1 nil] "ABC"]]
[:not [:contains [:field 1 nil] "XYZ"]]
[:not [:contains [:field 1 nil] "LMN"]]]
(mbql.u/desugar-filter-clause [:does-not-contain [:field 1 nil] "ABC" "XYZ" "LMN"]))))
(t/testing "*with* options"
(t/is (= [:and
[:not [:contains [:field 1 nil] "ABC" {:case-sensitive false}]]
[:not [:contains [:field 1 nil] "XYZ" {:case-sensitive false}]]]
(mbql.u/desugar-filter-clause
[:does-not-contain [:field 1 nil] "ABC" "XYZ" {:case-sensitive false}])))
(t/is (= [:and
[:not [:contains [:field 1 nil] "ABC" {:case-sensitive false}]]
[:not [:contains [:field 1 nil] "XYZ" {:case-sensitive false}]]
[:not [:contains [:field 1 nil] "LMN" {:case-sensitive false}]]]
(mbql.u/desugar-filter-clause
[:does-not-contain [:field 1 nil] "ABC" "XYZ" "LMN" {:case-sensitive false}])))))))

(t/deftest ^:parallel desugar-temporal-extract-test
(t/testing "desugaring :get-year, :get-month, etc"
Expand Down

0 comments on commit f17148b

Please sign in to comment.