From 4dd50562f762b909ff8c21200808565d66036a64 Mon Sep 17 00:00:00 2001
From: Ngoc Khuat <qn.khuat@gmail.com>
Date: Fri, 11 Nov 2022 13:55:04 +0700
Subject: [PATCH] fix mongo not able to filter by UUID (#26367)
---
.../metabase/driver/mongo/query_processor.clj | 34 +++++--
.../mongo/test/metabase/driver/mongo_test.clj | 89 ++++++++++++-------
2 files changed, 85 insertions(+), 38 deletions(-)
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 00b55c0d763..703d03b2971 100644
--- a/modules/drivers/mongo/src/metabase/driver/mongo/query_processor.clj
+++ b/modules/drivers/mongo/src/metabase/driver/mongo/query_processor.clj
@@ -25,7 +25,8 @@
$multiply $ne $not $or $project $regex $second $size $skip $sort $strcasecmp $subtract
$sum $toLower $year]]
[schema.core :as s])
- (:import org.bson.types.ObjectId))
+ (:import [org.bson.types ObjectId Binary]
+ org.bson.BsonBinarySubType))
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Schema |
@@ -289,15 +290,32 @@
(defmethod ->rvalue nil [_] nil)
+(defn- uuid->bsonbinary
+ [u]
+ (let [lo (.getLeastSignificantBits ^java.util.UUID u)
+ hi (.getMostSignificantBits ^java.util.UUID u)
+ ba (-> (java.nio.ByteBuffer/allocate 16) ; UUID is 128 bits-long
+ (.putLong hi)
+ (.putLong lo)
+ (.array))]
+ (Binary. BsonBinarySubType/UUID_STANDARD ba)))
+
(defmethod ->rvalue :value
[[_ value {base-type :base_type}]]
- (if (and (isa? base-type :type/MongoBSONID)
- (some? value))
- ;; Passing nil or "" to the ObjectId constructor throws an exception
- ;; "invalid hexadecimal representation of an ObjectId: []" so, just treat it as nil
- (when (not= value "")
- (ObjectId. (str value)))
- value))
+ (cond
+ ;; Passing nil or "" to the ObjectId or Binary constructor throws an exception
+ (or (nil? value) (= value ""))
+ value
+
+ (isa? base-type :type/MongoBSONID)
+ (ObjectId. (str value))
+
+ (isa? base-type :type/UUID)
+ (-> (str value)
+ java.util.UUID/fromString
+ uuid->bsonbinary)
+
+ :else value))
(defn- $date-from-string [s]
{:$dateFromString {:dateString (str s)}})
diff --git a/modules/drivers/mongo/test/metabase/driver/mongo_test.clj b/modules/drivers/mongo/test/metabase/driver/mongo_test.clj
index 444b453f8ac..1f3fbd4bd99 100644
--- a/modules/drivers/mongo/test/metabase/driver/mongo_test.clj
+++ b/modules/drivers/mongo/test/metabase/driver/mongo_test.clj
@@ -91,7 +91,7 @@
:dbname "metabase-test"
:version "2.2134234.lol"}
:expected false}]
- :let [ssl-details (tdm/conn-details details)]]
+ :let [ssl-details (tdm/conn-details details)]]
(testing (str "connect with " details)
(is (= expected
(let [db (db/insert! Database {:name "dummy", :engine "mongo", :details ssl-details})]
@@ -264,42 +264,71 @@
{:order-by [:name]})]
(into {} field))))))))))
-
(tx/defdataset with-bson-ids
[["birds"
[{:field-name "name", :base-type :type/Text}
- {:field-name "bird_id", :base-type :type/MongoBSONID}]
- [["Rasta Toucan" (ObjectId. "012345678901234567890123")]
- ["Lucky Pigeon" (ObjectId. "abcdefabcdefabcdefabcdef")]
+ {:field-name "bird_id", :base-type :type/MongoBSONID}
+ {:field-name "bird_uuid", :base-type :type/UUID}]
+ [["Rasta Toucan" (ObjectId. "012345678901234567890123") "11111111-1111-1111-1111-111111111111"]
+ ["Lucky Pigeon" (ObjectId. "abcdefabcdefabcdefabcdef") "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"]
["Unlucky Raven" nil]]]])
(deftest bson-ids-test
(mt/test-driver :mongo
- (testing "BSON IDs"
- (is (= [[2 "Lucky Pigeon" (ObjectId. "abcdefabcdefabcdefabcdef")]]
- (rows (mt/dataset with-bson-ids
- (mt/run-mbql-query birds
- {:filter [:= $bird_id "abcdefabcdefabcdefabcdef"]}))))
- "Check that we support Mongo BSON ID and can filter by it (#1367)")
-
- (is (= [[3 "Unlucky Raven" nil]]
- (rows (mt/dataset with-bson-ids
- (mt/run-mbql-query birds
- {:filter [:is-null $bird_id]}))))
- "handle null ObjectId queries properly (#11134)")
-
- (is (= [[3 "Unlucky Raven" nil]]
- (rows (mt/dataset with-bson-ids
- (mt/run-mbql-query birds
- {:filter [:is-empty $bird_id]}))))
- "treat null ObjectId as empty (#15801)")
-
- (is (= [[1 "Rasta Toucan" (ObjectId. "012345678901234567890123")]
- [2 "Lucky Pigeon" (ObjectId. "abcdefabcdefabcdefabcdef")]]
- (rows (mt/dataset with-bson-ids
- (mt/run-mbql-query birds
- {:filter [:not-empty $bird_id]}))))
- "treat non-null ObjectId as not-empty (#15801)"))))
+ (mt/dataset with-bson-ids
+ (testing "BSON IDs"
+ (testing "Check that we support Mongo BSON ID and can filter by it (#1367)"
+ (is (= [[2 "Lucky Pigeon" (ObjectId. "abcdefabcdefabcdefabcdef")]]
+ (rows (mt/run-mbql-query birds
+ {:filter [:= $bird_id "abcdefabcdefabcdefabcdef"]
+ :fields [$id $name $bird_id]})))))
+
+ (testing "handle null ObjectId queries properly (#11134)"
+ (is (= [[3 "Unlucky Raven" nil]]
+ (rows (mt/run-mbql-query birds
+ {:filter [:is-null $bird_id]
+ :fields [$id $name $bird_id]})))))
+
+ (testing "treat null ObjectId as empty (#15801)"
+ (is (= [[3 "Unlucky Raven" nil]]
+ (rows (mt/run-mbql-query birds
+ {:filter [:is-empty $bird_id]
+ :fields [$id $name $bird_id]})))))
+
+ (testing "treat non-null ObjectId as not-empty (#15801)"
+ (is (= [[1 "Rasta Toucan" (ObjectId. "012345678901234567890123")]
+ [2 "Lucky Pigeon" (ObjectId. "abcdefabcdefabcdefabcdef")]]
+ (rows (mt/run-mbql-query birds
+ {:filter [:not-empty $bird_id]
+ :fields [$id $name $bird_id]}))))))
+
+ (testing "BSON UUIDs"
+ (testing "Check that we support Mongo BSON UUID and can filter by it"
+ (is (= [[2 "Lucky Pigeon" "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"]]
+ (rows (mt/run-mbql-query birds
+ {:filter [:= $bird_uuid "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"]
+ :fields [$id $name $bird_uuid]})))))
+
+ (testing "handle null UUID queries properly"
+ (is (= [[3 "Unlucky Raven" nil]]
+ (rows (mt/run-mbql-query birds
+ {:filter [:is-null $bird_uuid]
+ :fields [$id $name $bird_uuid]})))))
+
+
+ (testing "treat null UUID as empty"
+ (is (= [[3 "Unlucky Raven" nil]]
+ (rows (mt/run-mbql-query birds
+ {:filter [:is-empty $bird_uuid]
+ :fields [$id $name $bird_uuid]}))))))
+
+ (testing "treat non-null UUID as not-empty"
+ (is (= [[1 "Rasta Toucan" "11111111-1111-1111-1111-111111111111"]
+ [2 "Lucky Pigeon" "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"]]
+ (rows (mt/run-mbql-query birds
+ {:filter [:not-empty $bird_uuid]
+ :fields [$id $name $bird_uuid]}))))))))
+
(deftest bson-fn-call-forms-test
(mt/test-driver :mongo
--
GitLab