diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj index 7d03f0801b35dee658bfac22e937d7a6dfcaf307..b1c403b12f5cab2240dd027735b93bcaf8c31c75 100644 --- a/src/metabase/driver.clj +++ b/src/metabase/driver.clj @@ -308,6 +308,7 @@ [java.util.UUID :type/Text] ; shouldn't this be :type/UUID ? [clojure.lang.IPersistentMap :type/Dictionary] [clojure.lang.IPersistentVector :type/Array] + [org.bson.types.ObjectId :type/MongoBSONID] [org.postgresql.util.PGobject :type/*]]) (log/warn (format "Don't know how to map class '%s' to a Field base_type, falling back to :type/*." klass)) :type/*)) diff --git a/src/metabase/driver/mongo/query_processor.clj b/src/metabase/driver/mongo/query_processor.clj index 655ccb93c2afd83ca0625828b8abbaaa3ec449f1..a315c7c5ae3a20365ba025184be6d624a7002244 100644 --- a/src/metabase/driver/mongo/query_processor.clj +++ b/src/metabase/driver/mongo/query_processor.clj @@ -148,6 +148,7 @@ 3]}) :year {$year field}))))) + (extend-protocol IRValue nil (->rvalue [_] nil) @@ -160,10 +161,9 @@ (str \$ (->lvalue this))) Value - (->rvalue [{value :value, {:keys [field-name base-type]} :field}] - (if (and (= field-name "_id") - (= base-type :type/*)) ; partial workaround for BSON ID Fields -- TODO fix this propertly (#1367) - `(ObjectId. ~value) + (->rvalue [{value :value, {:keys [base-type]} :field}] + (if (isa? base-type :type/MongoBSONID) + (ObjectId. (str value)) value)) DateTimeValue diff --git a/src/metabase/types.clj b/src/metabase/types.clj index ed3252c335de526a667a018679285af577bef88a..f940e69b691efb6b29a9144b7bdcc4b3284fb25e 100644 --- a/src/metabase/types.clj +++ b/src/metabase/types.clj @@ -59,10 +59,11 @@ (derive :type/Boolean :type/*) -;;; Text-Like Types: Things that should be displayed as text for most purposes but that shouldn't support advanced filter options like starts with / contains +;;; Text-Like Types: Things that should be displayed as text for most purposes but that *shouldn't* support advanced filter options like starts with / contains (derive :type/TextLike :type/*) (derive :type/IPAddress :type/TextLike) +(derive :type/MongoBSONID :type/TextLike) ;;; "Virtual" Types diff --git a/test/metabase/driver/mongo_test.clj b/test/metabase/driver/mongo_test.clj index 07ad99a68c42b495a15cada5e826fa74a4e33a75..f3475cbd35eb8e284df36d6ed05f8001a37007d7 100644 --- a/test/metabase/driver/mongo_test.clj +++ b/test/metabase/driver/mongo_test.clj @@ -7,9 +7,13 @@ [field :refer [Field]] [table :refer [Table] :as table]) [metabase.query-processor :as qp] + [metabase.query-processor.expand :as ql] + [metabase.query-processor-test :refer [rows]] [metabase.test.data :as data] - [metabase.test.data.datasets :as datasets]) - (:import metabase.driver.mongo.MongoDriver)) + [metabase.test.data.datasets :as datasets] + [metabase.test.data.interface :as i]) + (:import org.bson.types.ObjectId + metabase.driver.mongo.MongoDriver)) ;; ## Logic for selectively running mongo @@ -166,3 +170,19 @@ :table_id (:id (table-name->table nm)) {:order-by [:name]})] (into {} field)))) + + +;;; Check that we support Mongo BSON ID and can filter by it (#1367) + +(i/def-database-definition ^:private 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")]]]) + +(datasets/expect-with-engine :mongo + [[2 "Lucky Pigeon" (ObjectId. "abcdefabcdefabcdefabcdef")]] + (rows (data/dataset metabase.driver.mongo-test/with-bson-ids + (data/run-query birds + (ql/filter (ql/= $bird_id "abcdefabcdefabcdefabcdef")))))) diff --git a/test/metabase/test/data/mongo.clj b/test/metabase/test/data/mongo.clj index 22263d0b1590ee48222f4c09c438210119148f8f..883e5b588dd289d38c8a3e3e97016ceecc0bbf17 100644 --- a/test/metabase/test/data/mongo.clj +++ b/test/metabase/test/data/mongo.clj @@ -22,15 +22,14 @@ (destroy-db! dbdef) (with-mongo-connection [mongo-db (database->connection-details dbdef)] (doseq [{:keys [field-definitions table-name rows]} table-definitions] - (let [field-names (->> field-definitions - (map :field-name) - (map keyword))] + (let [field-names (for [field-definition field-definitions] + (keyword (:field-name field-definition)))] ;; Use map-indexed so we can get an ID for each row (index + 1) (doseq [[i row] (map-indexed (partial vector) rows)] (let [row (for [v row] ;; Conver all the java.sql.Timestamps to java.util.Date, because the Mongo driver insists on being obnoxious and going from ;; using Timestamps in 2.x to Dates in 3.x - (if (isa? (type v) java.sql.Timestamp) + (if (instance? java.sql.Timestamp v) (java.util.Date. (.getTime ^java.sql.Timestamp v)) v))] (try