diff --git a/.github/workflows/drivers.yml b/.github/workflows/drivers.yml
index fdf39b7e06939619b033733ebce00f0594d91412..a7de36c4b7150a074c2a4a9d333893569ad8e2ea 100644
--- a/.github/workflows/drivers.yml
+++ b/.github/workflows/drivers.yml
@@ -173,7 +173,7 @@ jobs:
with:
junit-name: 'be-tests-mariadb-latest-ee'
- be-tests-mongo-4-0-ee:
+ be-tests-mongo-4-2-ee:
if: github.event.pull_request.draft == false
runs-on: ubuntu-20.04
timeout-minutes: 60
@@ -184,17 +184,17 @@ jobs:
MB_MONGO_TEST_PASSWORD: metasample123
services:
mongodb:
- image: metabase/qa-databases:mongo-sample-4.0
+ image: metabase/qa-databases:mongo-sample-4.2
ports:
- "27017:27017"
steps:
- uses: actions/checkout@v3
- - name: Test MongoDB driver (4.0)
+ - name: Test MongoDB driver (4.2)
uses: ./.github/actions/test-driver
with:
- junit-name: 'be-tests-mongo-4-0-ee'
+ junit-name: 'be-tests-mongo-4-2-ee'
- be-tests-mongo-4-0-ssl-ee:
+ be-tests-mongo-4-2-ssl-ee:
if: github.event.pull_request.draft == false
runs-on: ubuntu-20.04
timeout-minutes: 60
@@ -207,7 +207,7 @@ jobs:
steps:
- uses: actions/checkout@v3
- name: Spin up Mongo docker container
- run: docker run -d -p 27017:27017 --name metamongo metabase/qa-databases:mongo-sample-4.0 mongod --dbpath /data/db2/ --sslMode requireSSL --sslPEMKeyFile /etc/mongo/metamongo.pem --sslCAFile /etc/mongo/metaca.crt
+ run: docker run -d -p 27017:27017 --name metamongo metabase/qa-databases:mongo-sample-4.2 mongod --dbpath /data/db2/ --sslMode requireSSL --sslPEMKeyFile /etc/mongo/metamongo.pem --sslCAFile /etc/mongo/metaca.crt
- name: Wait until the port 27017 is ready
run: while ! nc -z localhost 27017; do sleep 1; done
timeout-minutes: 5
@@ -222,10 +222,10 @@ jobs:
curl https://raw.githubusercontent.com/metabase/metabase-qa/master/dbs/mongo/certificates/metaca.crt \
-o ./test_resources/ssl/mongo/metaca.crt
- - name: Test MongoDB SSL driver (4.0)
+ - name: Test MongoDB SSL driver (4.2)
uses: ./.github/actions/test-driver
with:
- junit-name: 'be-tests-mongo-4-0-ee'
+ junit-name: 'be-tests-mongo-4-2-ee'
be-tests-mongo-5-0-ee:
if: github.event.pull_request.draft == false
diff --git a/docs/questions/query-builder/expressions-list.md b/docs/questions/query-builder/expressions-list.md
index 1b2a0cb5c84d43b6c5899e226293fdd32fbd51bc..05db2e33a001a03d3de992c4920b3fefbbf52a66 100644
--- a/docs/questions/query-builder/expressions-list.md
+++ b/docs/questions/query-builder/expressions-list.md
@@ -464,7 +464,7 @@ Related: [contains](#contains), [substring](#substring).
### replace
-Replaces a part of the input text with new text.
+Replaces all occurrences of a search text in the input text with the replacement text.
Syntax: `replace(text, find, replace)`.
diff --git a/modules/drivers/mongo/src/metabase/driver/mongo.clj b/modules/drivers/mongo/src/metabase/driver/mongo.clj
index a0e8d6abe4ed657fbbe3dbd6f2de2c94b1cd9f43..b469797dda808d01746345c693877bebef54cdbf 100644
--- a/modules/drivers/mongo/src/metabase/driver/mongo.clj
+++ b/modules/drivers/mongo/src/metabase/driver/mongo.clj
@@ -12,6 +12,7 @@
[metabase.driver.mongo.parameters :as mongo.params]
[metabase.driver.mongo.query-processor :as mongo.qp]
[metabase.driver.mongo.util :refer [with-mongo-connection]]
+ [metabase.driver.util :as driver.u]
[metabase.models :refer [Field]]
[metabase.query-processor.store :as qp.store]
[metabase.query-processor.timezone :as qp.timezone]
@@ -233,30 +234,24 @@
:standard-deviation-aggregations]]
(defmethod driver/supports? [:mongo feature] [_driver _feature] true))
-(defn- db-version [db]
- (get-in db [:details :version]))
+(defmethod driver/database-supports? [:mongo :expressions]
+ [_driver _feature db]
+ (-> (:dbms_version db)
+ :semantic-version
+ (driver.u/semantic-version-gte [4 2])))
-(defn- parse-version [version]
- (->> (str/split version #"\.")
- (take 2)
- (map #(Integer/parseInt %))))
-
-(defn- db-major-version [db]
- (some-> (db-version db) parse-version first))
-
-(defmethod driver/database-supports? [:mongo :expressions] [_ _ db]
- (let [version (db-major-version db)]
- (and (some? version) (>= version 4))))
-
-(defmethod driver/database-supports? [:mongo :date-arithmetics] [_ _ db]
- (let [version (db-major-version db)]
- (and (some? version) (>= version 5))))
+(defmethod driver/database-supports? [:mongo :date-arithmetics]
+ [_driver _feature db]
+ (-> (:dbms_version db)
+ :semantic-version
+ (driver.u/semantic-version-gte [5])))
(defmethod driver/database-supports? [:mongo :now]
;; The $$NOW aggregation expression was introduced in version 4.2.
- [_ _ db]
- (let [version (some-> (db-version db) parse-version)]
- (and (some? version) (>= (first version) 4) (>= (second version) 2))))
+ [_driver _feature db]
+ (-> (:dbms_version db)
+ :semantic-version
+ (driver.u/semantic-version-gte [4 2])))
(defmethod driver/mbql->native :mongo
[_ query]
@@ -320,16 +315,12 @@
(defmethod driver/table-rows-sample :mongo
[_driver table fields rff opts]
- (let [mongo-opts {;; setting :truncation-start is needed because of a bug
- ;; in our mongo drivers handling of :substring (#27270)
- :truncation-start 0
- :limit metadata-queries/nested-field-sample-limit
+ (let [mongo-opts {:limit metadata-queries/nested-field-sample-limit
:order-by [[:desc [:field (get-id-field-id table) nil]]]}]
(metadata-queries/table-rows-sample table fields rff (merge mongo-opts opts))))
(comment
(require '[clojure.java.io :as io]
- '[metabase.driver.util :as driver.u]
'[monger.credentials :as mcred])
(import javax.net.ssl.SSLSocketFactory)
diff --git a/modules/drivers/mongo/src/metabase/driver/mongo/query_processor.clj b/modules/drivers/mongo/src/metabase/driver/mongo/query_processor.clj
index d0b53ac9a31bb12a41918dca2224547e7ab350fc..802f3bbbd270104e4932000882396de2d070d595 100644
--- a/modules/drivers/mongo/src/metabase/driver/mongo/query_processor.clj
+++ b/modules/drivers/mongo/src/metabase/driver/mongo/query_processor.clj
@@ -8,6 +8,7 @@
[java-time :as t]
[metabase.driver :as driver]
[metabase.driver.common :as driver.common]
+ [metabase.driver.util :as driver.u]
[metabase.mbql.schema :as mbql.s]
[metabase.mbql.util :as mbql.u]
[metabase.models.field :refer [Field]]
@@ -20,13 +21,14 @@
[metabase.util.date-2 :as u.date]
[metabase.util.i18n :refer [tru]]
[metabase.util.schema :as su]
- [monger.operators :refer [$add $addToSet $and $avg $cond $dayOfMonth $dayOfWeek $dayOfYear $divide $eq
+ [monger.operators :refer [$add $addToSet $and $avg $cond
+ $dayOfMonth $dayOfWeek $dayOfYear $divide $eq
$group $gt $gte $hour $limit $lt $lte $match $max $min $minute $mod $month
$multiply $ne $not $or $project $regex $second $size $skip $sort $strcasecmp $subtract
$sum $toLower $year]]
[schema.core :as s])
- (:import [org.bson.types ObjectId Binary]
- org.bson.BsonBinarySubType))
+ (:import org.bson.BsonBinarySubType
+ [org.bson.types Binary ObjectId]))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Schema |
@@ -78,6 +80,9 @@
;; TODO - We already have a *query* dynamic var in metabase.query-processor.interface. Do we need this one too?
(def ^:dynamic ^:private *query* nil)
+(defn- get-mongo-version []
+ (driver/dbms-version :mongo (qp.store/database)))
+
(defmulti ^:private ->rvalue
"Format this `Field` or value for use as the right hand value of an expression, e.g. by adding `$` to a `Field`'s
name"
@@ -420,21 +425,62 @@
(defmethod ->rvalue :exp [[_ inp]] {"$exp" (->rvalue inp)})
(defmethod ->rvalue :sqrt [[_ inp]] {"$sqrt" (->rvalue inp)})
-(defmethod ->rvalue :trim [[_ inp]] {"$trim" (->rvalue inp)})
-(defmethod ->rvalue :ltrim [[_ inp]] {"$ltrim" (->rvalue inp)})
-(defmethod ->rvalue :rtrim [[_ inp]] {"$rtrim" (->rvalue inp)})
+(defmethod ->rvalue :trim [[_ inp]] {"$trim" {"input" (->rvalue inp)}})
+(defmethod ->rvalue :ltrim [[_ inp]] {"$ltrim" {"input" (->rvalue inp)}})
+(defmethod ->rvalue :rtrim [[_ inp]] {"$rtrim" {"input" (->rvalue inp)}})
(defmethod ->rvalue :upper [[_ inp]] {"$toUpper" (->rvalue inp)})
(defmethod ->rvalue :lower [[_ inp]] {"$toLower" (->rvalue inp)})
(defmethod ->rvalue :length [[_ inp]] {"$strLenCP" (->rvalue inp)})
(defmethod ->rvalue :power [[_ & args]] {"$pow" (mapv ->rvalue args)})
-(defmethod ->rvalue :replace [[_ & args]] {"$replaceAll" (mapv ->rvalue args)})
(defmethod ->rvalue :concat [[_ & args]] {"$concat" (mapv ->rvalue args)})
-(defmethod ->rvalue :substring [[_ & args]] {"$substrCP" (mapv ->rvalue args)})
-
(defmethod ->rvalue :temporal-extract [[_ inp unit]]
(with-rvalue-temporal-bucketing (->rvalue inp) unit))
+(defmethod ->rvalue :replace
+ [[_ & args]]
+ (let [version (get-mongo-version)]
+ (if (driver.u/semantic-version-gte (:semantic-version version) [4 4])
+ (let [[expr fnd replacement] (mapv ->rvalue args)]
+ {"$replaceAll" {"input" expr "find" fnd "replacement" replacement}})
+ (throw (ex-info "Replace requires MongoDB 4.4 or above"
+ {:database-version version})))))
+
+(defmethod ->rvalue :substring
+ [[_ & [expr idx cnt]]]
+ (let [expr-val (->rvalue expr)
+ idx-val {"$subtract" [(->rvalue idx) 1]}]
+ {"$substrCP" [expr-val
+ idx-val
+ ;; The last argument is not optional in mongo
+ (if (some? cnt)
+ (->rvalue cnt)
+ {"$subtract" [{"$strLenCP" expr-val} idx-val]})]}))
+
+(defmethod ->rvalue :/
+ [[_ & [_ & divisors :as args]]]
+ ;; division works outside in (/ 1 2 3) => (/ (/ 1 2) 3)
+ (let [division (reduce
+ (fn [accum head]
+ {"$divide" [accum head]})
+ (map ->rvalue args))
+ literal-zero? (some #(and (number? %) (zero? %)) divisors)
+ non-literal-nil-checks (mapv (fn [divisor] {"$eq" [(->rvalue divisor) 0]}) (remove number? divisors))]
+ (cond
+ literal-zero?
+ nil
+
+ (empty? non-literal-nil-checks)
+ division
+
+ (= 1 (count non-literal-nil-checks))
+ {"$cond" [(first non-literal-nil-checks) nil
+ division]}
+
+ :else
+ {"$cond" [{"$or" non-literal-nil-checks} nil
+ division]})))
+
;;; Intervals are not first class Mongo citizens, so they cannot be translated on their own.
;;; The only thing we can do with them is adding to or subtracting from a date valued expression.
;;; Also, date arithmetic with intervals was first implemented in version 5. (Before that only
@@ -443,9 +489,6 @@
;;; Because of this, whenever we translate date arithmetic with intervals, we check the major
;;; version of the database and throw a nice exception if it's less than 5.
-(defn- get-mongo-version []
- (driver/dbms-version :mongo (qp.store/database)))
-
(defn- check-date-operations-supported []
(let [{mongo-version :version, [major-version] :semantic-version} (get-mongo-version)]
(when (and major-version (< major-version 5))
@@ -494,7 +537,6 @@
{"$subtract" (mapv ->rvalue args)}))
(defmethod ->rvalue :* [[_ & args]] {"$multiply" (mapv ->rvalue args)})
-(defmethod ->rvalue :/ [[_ & args]] {"$divide" (mapv ->rvalue args)})
(defmethod ->rvalue :coalesce [[_ & args]] {"$ifNull" (mapv ->rvalue args)})
@@ -677,6 +719,10 @@
;;; -------------------------------------------------- aggregation ---------------------------------------------------
+(def ^:private aggregation-op
+ "The set of operators handled by [[aggregation->rvalue]] and [[expand-aggregation]]."
+ #{:avg :count :count-where :distinct :max :min :share :stddev :sum :sum-where :var})
+
(defmethod ->rvalue :case [[_ cases options]]
{:$switch {:branches (for [[pred expr] cases]
{:case (compile-cond pred)
@@ -685,8 +731,8 @@
(defn- aggregation->rvalue [ag]
(mbql.u/match-one ag
- [:aggregation-options ag _]
- (recur ag)
+ [:aggregation-options ag' _]
+ (recur ag')
[:count]
{$sum 1}
@@ -715,7 +761,7 @@
:else
(throw
(ex-info (tru "Don''t know how to handle aggregation {0}" ag)
- {:type :invalid-query, :clause ag}))))
+ {:type :invalid-query, :clause ag}))))
(defn- unwrap-named-ag [[ag-type arg :as ag]]
(if (= ag-type :aggregation-options)
@@ -755,41 +801,101 @@
(defmethod expand-aggregation :share
[[_ pred :as ag]]
- (let [count-where-name (name (gensym "count-where-"))
- count-name (name (gensym "count-"))
+ (let [count-where-expr (name (gensym "$count-where-"))
+ count-expr (name (gensym "$count-"))
pred (if (= (first pred) :share)
(second pred)
pred)]
- {:group {count-where-name (aggregation->rvalue [:count-where pred])
- count-name (aggregation->rvalue [:count])}
- :post {(annotate/aggregation-name ag) {$divide [(str "$" count-where-name) (str "$" count-name)]}}}))
+ {:group {(subs count-where-expr 1) (aggregation->rvalue [:count-where pred])
+ (subs count-expr 1) (aggregation->rvalue [:count])}
+ :post [{(annotate/aggregation-name ag) {$divide [count-where-expr count-expr]}}]}))
;; MongoDB doesn't have a variance operator, but you calculate it by taking the square of the standard deviation.
;; However, `$pow` is not allowed in the `$group` stage. So calculate standard deviation in the
(defmethod expand-aggregation :var
[ag]
(let [[_ expr] (unwrap-named-ag ag)
- stddev-name (name (gensym "stddev-"))]
- {:group {stddev-name (aggregation->rvalue [:stddev expr])}
- :post {(annotate/aggregation-name ag) {:$pow [(str \$ stddev-name) 2]}}}))
+ stddev-expr (name (gensym "$stddev-"))]
+ {:group {(subs stddev-expr 1) (aggregation->rvalue [:stddev expr])}
+ :post [{(annotate/aggregation-name ag) {:$pow [stddev-expr 2]}}]}))
(defmethod expand-aggregation :default
[ag]
{:group {(annotate/aggregation-name ag) (aggregation->rvalue ag)}})
+(defn- extract-aggregation
+ "Separate the expression `aggregation` named `aggr-name` into two parts:
+ an simple expression and an aggregation expression, where the simple expression
+ references the result of the aggregation expression such that first evaluating
+ the aggregation expression and binding its result to `aggr-name` and then
+ evaluating the simple expression in this context, the result is the same as
+ evaluating the whole expression `aggregation`.
+ This separation is necessary, because MongoDB doesn't support embedding
+ aggregations in `normal' expressions.
+
+ For example the aggregation
+ [:aggregation-options
+ [:+ [:/ [:sum
+ [:case [[[:< [:field 12 nil] [:field 7 nil]]
+ [:field 12 nil]]]
+ {:default 0}]]
+ 2]
+ 1]
+ {:name \"expression\"}]
+ is transformed into the simple expression
+ [:+ [:/ \"$expression\" 2] 1]
+ and the aggregation expression
+ [:aggregation-options
+ [:sum
+ [:case [[[:< [:field 12 nil] [:field 7 nil]]
+ [:field 12 nil]]]
+ {:default 0}]]
+ {:name \"expression\"}]"
+ [aggregation aggr-name]
+ (when (and (vector? aggregation) (seq aggregation))
+ (let [[op & args] aggregation]
+ (cond
+ (= op :aggregation-options)
+ (let [[embedding-expr aggregation'] (extract-aggregation (first args) aggr-name)]
+ [embedding-expr (into [:aggregation-options aggregation'] (rest args))])
+
+ (aggregation-op op)
+ [(str \$ aggr-name) aggregation]
+
+ :else
+ (let [ges (map #(extract-aggregation % aggr-name) args)
+ [embedding-expr aggregation'] (first (filter some? ges))]
+ (when-not aggregation'
+ (throw
+ (ex-info (tru "Don''t know how to handle aggregation {0}" aggregation)
+ {:type :invalid-query, :clause aggregation})))
+ [(into [op] (map (fn [arg ge] (if ge embedding-expr arg)) args ges))
+ aggregation'])))))
+
+(defn- expand-embedded-aggregation [aggregation]
+ (let [aggr-name (annotate/aggregation-name aggregation)
+ [embedding-expr aggregation-expr] (extract-aggregation aggregation aggr-name)
+ expanded (expand-aggregation aggregation-expr)]
+ (cond-> expanded
+ (not (string? embedding-expr))
+ (update :post conj {aggr-name (->rvalue embedding-expr)}))))
+
(defn- group-and-post-aggregations
"Mongo is picky about which top-level aggregations it allows with groups. Eg. even
though [:/ [:count-if ...] [:count]] is a perfectly fine reduction, it's not allowed. Therefore
more complex aggregations are split in two: the reductions are done in `$group` stage after which
- we do postprocessing in `$addFields` stage to arrive at the final result. The intermittent results
- accrued in `$group` stage are discarded in the final `$project` stage."
+ we do postprocessing in `$addFields` stage to arrive at the final result.
+ The groups are assumed to be independent an collapsed into a single stage, but separate
+ `$addFields` stages are created for post processing so that stages can refer to the results
+ of preceding stages.
+ The intermittent results accrued in `$group` stage are discarded in the final `$project` stage."
[id aggregations]
- (let [expanded-ags (map expand-aggregation aggregations)
+ (let [expanded-ags (map expand-embedded-aggregation aggregations)
group-ags (mapcat :group expanded-ags)
post-ags (mapcat :post expanded-ags)]
- [{$group (into (ordered-map/ordered-map "_id" id) group-ags)}
- (when (not-empty post-ags)
- {:$addFields (into (ordered-map/ordered-map) post-ags)})]))
+ (into [{$group (into (ordered-map/ordered-map "_id" id) group-ags)}]
+ (map (fn [p] {:$addFields p}))
+ post-ags)))
(defn- projection-group-map [fields]
(reduce
@@ -855,10 +961,6 @@
:asc 1
:desc -1)]))})
-(defn- handle-order-by [{:keys [order-by]} pipeline-ctx]
- (cond-> pipeline-ctx
- (seq order-by) (update :query conj (order-by->$sort order-by))))
-
;;; ----------------------------------------------------- fields -----------------------------------------------------
(defn- remove-parent-fields
@@ -869,8 +971,9 @@
To preserve the previous behavior, we will include only the child fields (since the parent field always appears first
in the projection/field order list, and that is the stated behavior according to the link above)."
[fields]
- (let [parent->child-id (reduce (fn [acc [_ field-id & _]]
- (if (integer? field-id)
+ (let [parent->child-id (reduce (fn [acc [agg-type field-id & _]]
+ (if (and (= agg-type :field)
+ (integer? field-id))
(let [field (qp.store/field field-id)]
(if-let [parent-id (:parent_id field)]
(update acc parent-id conj (u/the-id field))
@@ -882,6 +985,20 @@
(and (integer? field-id) (contains? parent->child-id field-id)))
fields)))
+(defn- handle-order-by [{:keys [order-by breakout]} pipeline-ctx]
+ (let [breakout-fields (set breakout)
+ sort-fields (for [field (remove-parent-fields (map second order-by))
+ ;; We only care about expressions not added as breakout
+ :when (and (not (contains? breakout-fields field))
+ (= :expression ((.dispatchFn ^clojure.lang.MultiFn ->rvalue) field)))]
+ [(->lvalue field) (->rvalue field)])]
+ (cond-> pipeline-ctx
+ (seq sort-fields) (update :query conj
+ ;; We $addFields before sorting, otherwise expressions will not be available for the sort
+ {:$addFields (into (ordered-map/ordered-map) sort-fields)})
+ (seq order-by) (update :query conj
+ (order-by->$sort order-by)))))
+
(defn- handle-fields [{:keys [fields]} pipeline-ctx]
(if-not (seq fields)
pipeline-ctx
diff --git a/modules/drivers/mongo/test/metabase/driver/mongo_test.clj b/modules/drivers/mongo/test/metabase/driver/mongo_test.clj
index 38362514ba22af1b6a63c35cd018d020cdb06683..4ca657bf7dc675cd3be26d41f5aaa261d062ba42 100644
--- a/modules/drivers/mongo/test/metabase/driver/mongo_test.clj
+++ b/modules/drivers/mongo/test/metabase/driver/mongo_test.clj
@@ -75,26 +75,18 @@
(deftest database-supports?-test
(mt/test-driver
:mongo
- (doseq [{:keys [details expected]} [{:details {:host "localhost"
- :port 3000
- :user "metabase"
- :pass "metasample123"
- :dbname "bad-db-name"
- :version "5.0.0"}
- :expected true}
- {:details {}
- :expected false}
- {:details {:version nil}
- :expected false}
- {:details {:host "localhost"
- :port 27017
- :dbname "metabase-test"
- :version "2.2134234.lol"}
- :expected false}]
- :let [ssl-details (tdm/conn-details details)]]
- (testing (str "connect with " details)
+ (doseq [{:keys [dbms_version expected]}
+ [{:dbms_version {:semantic-version [5 0 0 0]}
+ :expected true}
+ {:dbms_version {}
+ :expected false}
+ {:dbms_version {:semantic-version []}
+ :expected false}
+ {:dbms_version {:semantic-version [2 2134234]}
+ :expected false}]]
+ (testing (str "supports with " dbms_version)
(is (= expected
- (let [db (db/insert! Database {:name "dummy", :engine "mongo", :details ssl-details})]
+ (let [db (db/insert! Database {:name "dummy", :engine "mongo", :dbms_version dbms_version})]
(driver/database-supports? :mongo :expressions db))))))))
diff --git a/src/metabase/db/metadata_queries.clj b/src/metabase/db/metadata_queries.clj
index 34470646941566fc34acc33d8ee2fe2ddaaed6b8..3291f361caa93d05c11f84e42ee87ac5fe609ee9 100644
--- a/src/metabase/db/metadata_queries.clj
+++ b/src/metabase/db/metadata_queries.clj
@@ -89,8 +89,7 @@
(def TableRowsSampleOptions
"Schema for `table-rows-sample` options"
- (s/maybe {(s/optional-key :truncation-start) s/Int
- (s/optional-key :truncation-size) s/Int
+ (s/maybe {(s/optional-key :truncation-size) s/Int
(s/optional-key :limit) s/Int
(s/optional-key :order-by) (helpers/distinct (helpers/non-empty [mbql.s/OrderBy]))
(s/optional-key :rff) s/Any}))
@@ -108,9 +107,7 @@
"Returns the mbql query to query a table for sample rows"
[table
fields
- {:keys [truncation-start truncation-size limit order-by]
- :or {truncation-start 1, limit max-sample-rows}
- :as _opts}]
+ {:keys [truncation-size limit order-by] :or {limit max-sample-rows} :as _opts}]
(let [database (table/database table)
driver (driver.u/database->driver database)
text-fields (filter text-field? fields)
@@ -118,7 +115,7 @@
(into {} (for [field text-fields]
[field [(str (gensym "substring"))
[:substring [:field (u/the-id field) nil]
- truncation-start truncation-size]]])))]
+ 1 truncation-size]]])))]
{:database (:db_id table)
:type :query
:query (cond-> {:source-table (u/the-id table)
diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj
index be6178f1a404cdf53e20b462869770c2d0b656fe..4e8acb97c0676a97dbd38934c333d958261b2b41 100644
--- a/src/metabase/driver.clj
+++ b/src/metabase/driver.clj
@@ -515,9 +515,8 @@
(e.g., :left-join is not supported by any version of Mongo DB).
In some cases, a feature may only be supported by certain versions of the database engine.
- In this case, after implementing `:version` in `describe-database` for the driver,
- you can check in `(get-in db [:details :version])` and determine
- whether a feature is supported for this particular database.
+ In this case, after implementing `[[dbms-version]]` for your driver
+ you can determine whether a feature is supported for this particular database.
(database-supports? :mongo :set-timezone mongo-db) ; -> true"
{:arglists '([driver feature database]), :added "0.41.0"}
diff --git a/src/metabase/driver/util.clj b/src/metabase/driver/util.clj
index 8f0932398dace00464a0612aa2c58580a4668b76..f77157b155516461ac4760f46d13aee1a745283a 100644
--- a/src/metabase/driver/util.clj
+++ b/src/metabase/driver/util.clj
@@ -13,6 +13,8 @@
[metabase.query-processor.error-type :as qp.error-type]
[metabase.util :as u]
[metabase.util.i18n :refer [deferred-tru trs]]
+ [metabase.util.schema :as su]
+ [schema.core :as s]
[toucan.db :as db])
(:import
(java.io ByteArrayInputStream)
@@ -160,16 +162,6 @@
(log/error e (trs "Failed to connect to database"))
false))))
-(defn report-timezone-if-supported
- "Returns the report-timezone if `driver` supports setting it's timezone and a report-timezone has been specified by
- the user."
- [driver]
- (when (driver/supports? driver :set-timezone)
- (let [report-tz (driver/report-timezone)]
- (when (seq report-tz)
- report-tz))))
-
-
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Driver Resolution |
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -212,6 +204,25 @@
:when (driver/available? driver)]
driver)))
+(s/defn semantic-version-gte
+ "Returns true if xv is greater than or equal to yv according to semantic versioning.
+ xv and yv are sequences of integers of the form `[major minor ...]`, where only
+ major is obligatory.
+ Examples:
+ (semantic-version-gte [4 1] [4 1]) => true
+ (semantic-version-gte [4 0 1] [4 1]) => false
+ (semantic-version-gte [4 1] [4]) => true
+ (semantic-version-gte [3 1] [4]) => false"
+ [xv :- [su/NonNegativeInt] yv :- [su/NonNegativeInt]] :- s/Bool
+ (loop [xv (seq xv), yv (seq yv)]
+ (or (nil? yv)
+ (let [[x & xs] xv
+ [y & ys] yv
+ x (if (nil? x) 0 x)
+ y (if (nil? y) 0 y)]
+ (or (> x y)
+ (and (>= x y) (recur xs ys)))))))
+
(defn- file-upload-props [{prop-name :name, visible-if :visible-if, disp-nm :display-name, :as conn-prop}]
(if (premium-features/is-hosted?)
[(-> (assoc conn-prop
diff --git a/src/metabase/query_processor/store.clj b/src/metabase/query_processor/store.clj
index 46cb7f5001f836578e2e53fdfc536203d7bfe3d7..5148d29bf047232af73d9dd296e523a256ada26c 100644
--- a/src/metabase/query_processor/store.clj
+++ b/src/metabase/query_processor/store.clj
@@ -57,6 +57,7 @@
[:id
:engine
:name
+ :dbms_version
:details
:settings])
diff --git a/test/metabase/driver/util_test.clj b/test/metabase/driver/util_test.clj
index a51ab747067b26e1696fb45c3cbb6317650ba831..6708f7b6d395d1ae7fa675d29c2509a50b321712 100644
--- a/test/metabase/driver/util_test.clj
+++ b/test/metabase/driver/util_test.clj
@@ -235,3 +235,20 @@
(select-keys transformed [:host :password-value :keystore-password-value :use-keystore])))
;; the keystore-value should have been base64 decoded because of treat-before-posting being base64 (see above)
(is (mt/secret-value-equals? ks-val (:keystore-value transformed))))))
+
+(deftest semantic-version-gte-test
+ (testing "semantic-version-gte works as expected"
+ (is (true? (driver.u/semantic-version-gte [5 0] [4 0])))
+ (is (true? (driver.u/semantic-version-gte [5 0 1] [4 0])))
+ (is (true? (driver.u/semantic-version-gte [5 0] [4 0 1])))
+ (is (true? (driver.u/semantic-version-gte [5 0] [4 1])))
+ (is (true? (driver.u/semantic-version-gte [4 1] [4 1])))
+ (is (true? (driver.u/semantic-version-gte [4 1] [4])))
+ (is (true? (driver.u/semantic-version-gte [4] [4])))
+ (is (true? (driver.u/semantic-version-gte [4] [4 0 0])))
+ (is (false? (driver.u/semantic-version-gte [3] [4])))
+ (is (false? (driver.u/semantic-version-gte [4] [4 1])))
+ (is (false? (driver.u/semantic-version-gte [4 0] [4 0 1])))
+ (is (false? (driver.u/semantic-version-gte [4 0 1] [4 1])))
+ (is (false? (driver.u/semantic-version-gte [3 9] [4 0])))
+ (is (false? (driver.u/semantic-version-gte [3 1] [4])))))
diff --git a/test/metabase/query_processor_test/advanced_math_test.clj b/test/metabase/query_processor_test/advanced_math_test.clj
index 77996b65c77ff3972bfa8a79c32b51b76c3bbd98..e8b475b34921ab86ea80410db7c04c9b06579060 100644
--- a/test/metabase/query_processor_test/advanced_math_test.clj
+++ b/test/metabase/query_processor_test/advanced_math_test.clj
@@ -2,6 +2,7 @@
(:require
[clojure.test :refer :all]
[metabase.driver :as driver]
+ [metabase.driver.util :as driver.u]
[metabase.test :as mt]
[metabase.util :as u]))
@@ -21,7 +22,13 @@
(deftest test-round
(mt/test-drivers (mt/normal-drivers-with-feature :expressions)
- (is (= 1.0 (test-math-expression [:round 0.7])))))
+ (if (or (not= driver/*driver* :mongo)
+ ;; mongo supports $round since version 4.2
+ (driver.u/semantic-version-gte
+ (-> (mt/db) :dbms_version :semantic-version)
+ [4 2]))
+ (is (= 1.0 (test-math-expression [:round 0.7])))
+ (is (= 0 0)))))
(deftest test-floor
(mt/test-drivers (mt/normal-drivers-with-feature :expressions)
diff --git a/test/metabase/query_processor_test/date_time_zone_functions_test.clj b/test/metabase/query_processor_test/date_time_zone_functions_test.clj
index 0f65da4581d8a10f486bd9abd7b215824ddae5fe..158a28892c319ac41b9df3fa6e352b2290d1ef84 100644
--- a/test/metabase/query_processor_test/date_time_zone_functions_test.clj
+++ b/test/metabase/query_processor_test/date_time_zone_functions_test.clj
@@ -226,7 +226,7 @@
(is (= expected (test-temporal-extract query))))))))
(deftest temporal-extraction-with-datetime-arithmetic-expression-tests
- (mt/test-drivers (mt/normal-drivers-with-feature :temporal-extract :expressions)
+ (mt/test-drivers (mt/normal-drivers-with-feature :temporal-extract :expressions :date-arithmetics)
(mt/dataset times-mixed
(doseq [{:keys [title expected query]}
[{:title "Nested interval addition expression"
diff --git a/test/metabase/query_processor_test/expressions_test.clj b/test/metabase/query_processor_test/expressions_test.clj
index b8db4092a85c8b652d7015bde7edd609750bb549..68c329858475d4d782089984a7d0de7f6e5ed030 100644
--- a/test/metabase/query_processor_test/expressions_test.clj
+++ b/test/metabase/query_processor_test/expressions_test.clj
@@ -130,7 +130,16 @@
(mt/run-mbql-query venues
{:expressions {:x [:+ $price $id]}
:limit 3
- :order-by [[:desc [:expression :x]]]})))))))
+ :order-by [[:desc [:expression :x]]]})))))
+ (testing "Can we refer to expressions inside an ORDER BY clause with a secondary order by?"
+ (is (= [[81 "Tanoshi Sushi & Sake Bar" 40 40.7677 -73.9533 4 85.0]
+ [79 "Sushi Yasuda" 40 40.7514 -73.9736 4 83.0]
+ [77 "Sushi Nakazawa" 40 40.7318 -74.0045 4 81.0]]
+ (mt/formatted-rows [int str int 4.0 4.0 int float]
+ (mt/run-mbql-query venues
+ {:expressions {:x [:+ $price $id]}
+ :limit 3
+ :order-by [[:desc $price] [:desc [:expression :x]]]})))))))
(deftest aggregate-breakout-expression-test
(mt/test-drivers (mt/normal-drivers-with-feature :expressions)
@@ -195,12 +204,11 @@
(is (= [[nil] [0.0] [0.0] [10.0] [8.0] [5.0] [5.0] [nil] [0.0] [0.0]]
(calculate-bird-scarcity $count))))
- (testing (str "do expressions automatically handle division by zero? Should return `nil` in the results for places "
- "where that was attempted")
- (is (= [[nil] [nil] [10.0] [12.5] [20.0] [20.0] [nil] [nil] [9.09] [7.14]]
- (calculate-bird-scarcity [:/ 100.0 $count]
- [:!= $count nil]))))
-
+ (testing (str "do expressions automatically handle division by zero? Should return `nil` "
+ "in the results for places where that was attempted")
+ (is (= [[nil] [nil] [10.0] [12.5] [20.0] [20.0] [nil] [nil] [9.09] [7.14]]
+ (calculate-bird-scarcity [:/ 100.0 $count]
+ [:!= $count nil]))))
(testing (str "do expressions handle division by `nil`? Should return `nil` in the results for places where that "
"was attempted")
@@ -211,16 +219,20 @@
[:!= $count 0]]))))
(testing "can we handle BOTH NULLS AND ZEROES AT THE SAME TIME????"
- (is (= [[nil] [nil] [nil] [10.0] [12.5] [20.0] [20.0] [nil] [nil] [nil]]
- (calculate-bird-scarcity [:/ 100.0 $count]))))
+ (is (= [[nil] [nil] [nil] [10.0] [12.5] [20.0] [20.0] [nil] [nil] [nil]]
+ (calculate-bird-scarcity [:/ 100.0 $count]))))
+
+ (testing "can we handle dividing by literal 0?"
+ (is (= [[nil] [nil] [nil] [nil] [nil] [nil] [nil] [nil] [nil] [nil]]
+ (calculate-bird-scarcity [:/ $count 0]))))
(testing "ok, what if we use multiple args to divide, and more than one is zero?"
- (is (= [[nil] [nil] [nil] [1.0] [1.56] [4.0] [4.0] [nil] [nil] [nil]]
- (calculate-bird-scarcity [:/ 100.0 $count $count]))))
+ (is (= [[nil] [nil] [nil] [1.0] [1.56] [4.0] [4.0] [nil] [nil] [nil]]
+ (calculate-bird-scarcity [:/ 100.0 $count $count]))))
(testing "are nulls/zeroes still handled appropriately when nested inside other expressions?"
- (is (= [[nil] [nil] [nil] [20.0] [25.0] [40.0] [40.0] [nil] [nil] [nil]]
- (calculate-bird-scarcity [:* [:/ 100.0 $count] 2]))))
+ (is (= [[nil] [nil] [nil] [20.0] [25.0] [40.0] [40.0] [nil] [nil] [nil]]
+ (calculate-bird-scarcity [:* [:/ 100.0 $count] 2]))))
(testing (str "if a zero is present in the NUMERATOR we should return ZERO and not NULL "
"(`0 / 10 = 0`; `10 / 0 = NULL`, at least as far as MBQL is concerned)")
@@ -235,7 +247,6 @@
(is (= [[nil] [10.0] [10.0] [0.0] [2.0] [5.0] [5.0] [nil] [10.0] [10.0]]
(calculate-bird-scarcity [:- 10 $count]))))
-
(testing "can multiplications handle nulls & zeros?"
(is (= [[nil] [0.0] [0.0] [10.0] [8.0] [5.0] [5.0] [nil] [0.0] [0.0]]
(calculate-bird-scarcity [:* 1 $count]))))))
@@ -255,7 +266,7 @@
[(format-fn (u.date/parse s "UTC"))])))
(deftest temporal-arithmetic-test
- (mt/test-drivers (mt/normal-drivers-with-feature :expressions)
+ (mt/test-drivers (mt/normal-drivers-with-feature :expressions :date-arithmetics)
(testing "Test that we can do datetime arithemtics using MBQL `:interval` clause in expressions"
(is (= (robust-dates
["2014-09-02T13:45:00"
@@ -377,7 +388,9 @@
:order-by [[:asc $id]]})))))))
(deftest expression-using-aggregation-test
- (mt/test-drivers (mt/normal-drivers-with-feature :expressions)
+ (mt/test-drivers (disj (mt/normal-drivers-with-feature :expressions)
+ ;; The limit in source-query is ignored (#27249)
+ :mongo)
(testing "Can we use aggregations from previous steps in expressions (#12762)"
(is (= [["20th Century Cafe" 2 2 0]
[ "25°" 2 2 0]
diff --git a/test/metabase/query_processor_test/filter_test.clj b/test/metabase/query_processor_test/filter_test.clj
index ac00ccdc0a426755c5592f3a2ee645fb5dc02bdf..e59b9d0e46d6f94d28a441a72904d6f501ea0e34 100644
--- a/test/metabase/query_processor_test/filter_test.clj
+++ b/test/metabase/query_processor_test/filter_test.clj
@@ -112,25 +112,17 @@
{:aggregation [[:count]]
:filter [:between !day.date "2015-04-01" "2015-05-01"]}))))))))
-(defn- mongo-major-version [db]
- (when (= driver/*driver* :mongo)
- (-> (driver/dbms-version :mongo db) :semantic-version first)))
-
(defn- timezone-arithmetic-drivers []
(set/intersection
- ;; we also want to test this against MongoDB but [[mt/normal-drivers-with-feature]] would normally not include that
- ;; since MongoDB only supports expressions if version is 4.0 or above and [[mt/normal-drivers-with-feature]]
- ;; currently uses [[driver/supports?]] rather than [[driver/database-supports?]] (TODO FIXME, see #23422)
- (conj (mt/normal-drivers-with-feature :expressions) :mongo)
- (timezones-test/timezone-aware-column-drivers)))
+ (mt/normal-drivers-with-feature :expressions)
+ (mt/normal-drivers-with-feature :date-arithmetics)
+ (timezones-test/timezone-aware-column-drivers)))
(deftest temporal-arithmetic-test
(testing "Should be able to use temporal arithmetic expressions in filters (#22531)"
(mt/test-drivers (timezone-arithmetic-drivers)
(mt/dataset attempted-murders
- (when-not (some-> (mongo-major-version (mt/db))
- (< 5))
- (doseq [offset-unit [:year :day]
+ (doseq [offset-unit [:year :day]
interval-unit [:year :day]
compare-op [:between := :< :<= :> :>=]
compare-order (cond-> [:field-first]
@@ -154,15 +146,13 @@
(if (= driver/*driver* :mongo)
(is (or (nil? result)
(pos-int? result)))
- (is (nat-int? result))))))))))))
+ (is (nat-int? result)))))))))))
(deftest nonstandard-temporal-arithmetic-test
(testing "Nonstandard temporal arithmetic should also be supported"
(mt/test-drivers (timezone-arithmetic-drivers)
(mt/dataset attempted-murders
- (when-not (some-> (mongo-major-version (mt/db))
- (< 5))
- (doseq [offset-unit [:year :day]
+ (doseq [offset-unit [:year :day]
interval-unit [:year :day]
compare-op [:between := :< :<= :> :>=]
add-op [:+ #_:-] ; TODO support subtraction like sql.qp/add-interval-honeysql-form (#23423)
@@ -191,7 +181,7 @@
(if (= driver/*driver* :mongo)
(is (or (nil? result)
(pos-int? result)))
- (is (nat-int? result))))))))))))
+ (is (nat-int? result)))))))))))
(deftest or-test
(mt/test-drivers (mt/normal-drivers)
diff --git a/test/metabase/query_processor_test/string_extracts_test.clj b/test/metabase/query_processor_test/string_extracts_test.clj
index f0eff63e8cbbb0ecdc77c1cb62239d5e665810ca..401514b8bef53d491a83a7ad102f63b7116770e1 100644
--- a/test/metabase/query_processor_test/string_extracts_test.clj
+++ b/test/metabase/query_processor_test/string_extracts_test.clj
@@ -1,6 +1,8 @@
(ns metabase.query-processor-test.string-extracts-test
(:require
[clojure.test :refer :all]
+ [metabase.driver :as driver]
+ [metabase.driver.util :as driver.u]
[metabase.query-processor :as qp]
[metabase.test :as mt]
[metabase.test.data :as data]))
@@ -47,11 +49,24 @@
(is (= "Red" (test-string-extract [:substring [:field (data/id :venues :name) nil] 1 3])))
(is (= "ed Medicine" (test-string-extract [:substring [:field (data/id :venues :name) nil] 2])))
(is (= "Red Medicin" (test-string-extract [:substring [:field (data/id :venues :name) nil]
- 1 [:- [:length [:field (data/id :venues :name) nil]] 1]])))))
+ 1 [:- [:length [:field (data/id :venues :name) nil]] 1]])))
+ (is (= "ne" (test-string-extract [:substring [:field (data/id :venues :name) nil]
+ [:- [:length [:field (data/id :venues :name) nil]] 1]])))))
(deftest test-replace
(mt/test-drivers (mt/normal-drivers-with-feature :expressions)
- (is (= "Red Baloon" (test-string-extract [:replace [:field (data/id :venues :name) nil] "Medicine" "Baloon"])))))
+ (when
+ (or (not= driver/*driver* :mongo)
+ ;; mongo supports $replaceAll since version 4.4
+ (driver.u/semantic-version-gte
+ (-> (mt/db) :dbms_version :semantic-version)
+ [4 4]))
+ (is (= "Red Baloon" (test-string-extract [:replace [:field (data/id :venues :name) nil] "Medicine" "Baloon"])))
+ (is (= "Rod Modicino" (test-string-extract [:replace [:field (data/id :venues :name) nil] "e" "o"])))
+ (is (= "Red" (test-string-extract [:replace [:field (data/id :venues :name) nil] " Medicine" ""])))
+ (is (= "Larry's The Prime Rib" (test-string-extract
+ [:replace [:field (data/id :venues :name) nil] "Lawry's" "Larry's"]
+ [:= [:field (data/id :venues :name) nil] "Lawry's The Prime Rib"]))))))
(deftest test-coalesce
(mt/test-drivers (mt/normal-drivers-with-feature :expressions)
@@ -84,13 +99,6 @@
(mt/formatted-rows [identity int])
first)))))
-(deftest replace-escaping-test
- (mt/test-drivers
- (mt/normal-drivers-with-feature :expressions)
- (is (= "Larry's The Prime Rib" (test-string-extract
- [:replace [:field (data/id :venues :name) nil] "Lawry's" "Larry's"]
- [:= [:field (data/id :venues :name) nil] "Lawry's The Prime Rib"])))))
-
(deftest regex-match-first-escaping-test
(mt/test-drivers
(mt/normal-drivers-with-feature :expressions :regex)