From bc397e1ab02d5b0d59c49abb85df9f8dffa46761 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cam=20Sau=CC=88l?= <cammsaul@gmail.com>
Date: Thu, 9 Jul 2015 15:38:33 -0700
Subject: [PATCH] Everything is finally working :heart_eyes:
---
project.clj | 3 +-
src/metabase/db.clj | 8 ++++-
src/metabase/db/internal.clj | 11 ++++++
src/metabase/driver.clj | 36 ++++++++++---------
src/metabase/driver/generic_sql/native.clj | 6 +---
src/metabase/driver/mongo.clj | 3 +-
src/metabase/driver/mongo/query_processor.clj | 6 ++--
src/metabase/driver/mongo/util.clj | 11 +++---
src/metabase/driver/sync.clj | 29 +++++++++------
9 files changed, 68 insertions(+), 45 deletions(-)
diff --git a/project.clj b/project.clj
index 081323525e4..890036e7ce1 100644
--- a/project.clj
+++ b/project.clj
@@ -59,7 +59,8 @@
:init metabase.core/init}
:eastwood {:exclude-namespaces [:test-paths]
:add-linters [:unused-private-vars]
- :exclude-linters [:constant-test]} ; korma macros generate some formats with if statements that are always logically true or false
+ :exclude-linters [:constant-test ; korma macros generate some forms with if statements that are always logically true or false
+ :suspicious-expression]} ; core.match macros generate some forms like (and expr) which is "suspicious"
:profiles {:dev {:dependencies [[org.clojure/tools.nrepl "0.2.10"] ; REPL <3
[expectations "2.1.1"] ; unit tests
[marginalia "0.8.0"] ; for documentation
diff --git a/src/metabase/db.clj b/src/metabase/db.clj
index ae11a585774..399d83338af 100644
--- a/src/metabase/db.clj
+++ b/src/metabase/db.clj
@@ -186,7 +186,8 @@
(sel :one User :id 1) -> returns the User (or nil) whose id is 1
(sel :many OrgPerm :user_id 1) -> returns sequence of OrgPerms whose user_id is 1
- OPTION, if specified, is one of `:field`, `:fields`, `:id`, `:id->field`, `:field->id`, `:field->obj`, or `:id->fields`.
+ OPTION, if specified, is one of `:field`, `:fields`, `:id`, `:id->field`, `:field->id`, `:field->obj`, `:id->fields`,
+ `:field->field`, or `:field->fields`.
;; Only return IDs of objects.
(sel :one :id User :email \"cam@metabase.com\") -> 120
@@ -215,6 +216,11 @@
-> {\"venues\" {:id 1, :name \"venues\", ...}
\"users\" {:id 2, :name \"users\", ...}}
+ ;; Return a map of field value -> other fields.
+ (sel :many :field->fields [Table :name :id :db_id])
+ -> {\"venues\" {:id 1, :db_id 1}
+ \"users\" {:id 2, :db_id 1}}
+
;; Return a map of ID -> specified fields
(sel :many :id->fields [User :first_name :last_name])
-> {1 {:first_name \"Cam\", :last_name \"Saul\"},
diff --git a/src/metabase/db/internal.clj b/src/metabase/db/internal.clj
index fb7c7fc0c9e..8f8cfe6e1f4 100644
--- a/src/metabase/db/internal.clj
+++ b/src/metabase/db/internal.clj
@@ -137,6 +137,17 @@
f2# ~f2]
(sel:field->field* f1# f2# (sel* [~entity f1# f2#] ~@forms))))
+;;; :field->fields
+
+(defn sel:field->fields* [key-field other-fields results]
+ (into {} (for [result results]
+ {(key-field result) (select-keys result other-fields)})))
+
+(defmacro sel:field->fields [[entity key-field & other-fields] & forms]
+ `(let [key-field# ~key-field
+ other-fields# ~(vec other-fields)]
+ (sel:field->fields* key-field# other-fields# (sel* `[~~entity ~key-field# ~@other-fields#] ~@forms))))
+
;;; : id->field
(defmacro sel:id->field [[entity field] & forms]
diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj
index 9f8e32701a5..b00dc023556 100644
--- a/src/metabase/driver.clj
+++ b/src/metabase/driver.clj
@@ -32,22 +32,26 @@
:name "MongoDB"
:example "mongodb://password:username@127.0.0.1:27017/db-name"}})
-(def ^:const class->base-type
- "Map of classes returned from DB call to metabase.models.field/base-types"
- {clojure.lang.PersistentArrayMap :DictionaryField ; are there other map types we need to account for ?
- Boolean :BooleanField
- Double :FloatField
- Float :FloatField
- Integer :IntegerField
- Long :IntegerField
- String :TextField
- java.math.BigDecimal :DecimalField
- java.math.BigInteger :BigIntegerField
- java.sql.Date :DateField
- java.sql.Timestamp :DateTimeField
- java.util.Date :DateField
- java.util.UUID :TextField
- org.postgresql.util.PGobject :UnknownField}) ; this mapping included here since Native QP uses class->base-type directly. TODO - perhaps make *class-base->type* driver specific?
+(defn class->base-type
+ "Return the `Field.base_type` that corresponds to a given class returned by the DB."
+ [klass]
+ (or ({Boolean :BooleanField
+ Double :FloatField
+ Float :FloatField
+ Integer :IntegerField
+ Long :IntegerField
+ String :TextField
+ java.math.BigDecimal :DecimalField
+ java.math.BigInteger :BigIntegerField
+ java.sql.Date :DateField
+ java.sql.Timestamp :DateTimeField
+ java.util.Date :DateField
+ java.util.UUID :TextField
+ org.postgresql.util.PGobject :UnknownField} klass)
+ (cond
+ (isa? klass clojure.lang.IPersistentMap) :DictionaryField)
+ (do (log/warn (format "Don't know how to map class '%s' to a Field base_type, falling back to :UnknownField." klass))
+ :UnknownField)))
;; ## Driver Lookup
diff --git a/src/metabase/driver/generic_sql/native.clj b/src/metabase/driver/generic_sql/native.clj
index cd36f36f5b5..146fcb203b1 100644
--- a/src/metabase/driver/generic_sql/native.clj
+++ b/src/metabase/driver/generic_sql/native.clj
@@ -13,11 +13,7 @@
(defn- value->base-type
"Attempt to match a value we get back from the DB with the corresponding base-type`."
[v]
- (if-not v :UnknownField
- (or (driver/class->base-type (type v))
- (do (log/warn (format "Missing base type mapping for %s in driver/class->base-type. Please add an entry."
- (str (type v))))
- :UnknownField))))
+ (driver/class->base-type (type v)))
(defn process-and-run
"Process and run a native (raw SQL) QUERY."
diff --git a/src/metabase/driver/mongo.clj b/src/metabase/driver/mongo.clj
index 52c4421a158..826c97fe99f 100644
--- a/src/metabase/driver/mongo.clj
+++ b/src/metabase/driver/mongo.clj
@@ -132,8 +132,7 @@
(sort-by second) ; source by count
last ; take last item (highest count)
first ; keep just the type
- (#(or (driver/class->base-type %) ; convert to corresponding Field base_type if possible
- :UnknownField))))))))) ; fall back to :UnknownField for things like clojure.lang.PersistentVector
+ driver/class->base-type))))))) ; get corresponding Field base_type
(def driver
"Concrete instance of the MongoDB driver."
diff --git a/src/metabase/driver/mongo/query_processor.clj b/src/metabase/driver/mongo/query_processor.clj
index 76b42e2694b..e663381ffc2 100644
--- a/src/metabase/driver/mongo/query_processor.clj
+++ b/src/metabase/driver/mongo/query_processor.clj
@@ -213,9 +213,9 @@
is present, this is essentialy a separate implementation :/"
[{aggregation :aggregation, breakout-fields :breakout, order-by :order-by, limit :limit, :as query}]
(let [;; Shadow the top-level definition of field->name with one that will use "___" as the separator instead of "."
- field->name (u/rpartial field->name "___")
+ field->escaped-name (u/rpartial field->name "___")
[ag-field ag-clause] (breakout-aggregation->field-name+expression aggregation)
- fields (map field->name breakout-fields)
+ fields (map field->escaped-name breakout-fields)
$fields (map field->$str breakout-fields)
fields->$fields (zipmap fields $fields)]
`(ag-unescape-nested-field-names
@@ -228,7 +228,7 @@
{field {$first $field}}))))}
{$sort (->> order-by
(mapcat (fn [{:keys [field direction]}]
- [(field->name field) (case direction
+ [(field->escaped-name field) (case direction
:ascending 1
:descending -1)]))
(apply sorted-map))}
diff --git a/src/metabase/driver/mongo/util.clj b/src/metabase/driver/mongo/util.clj
index 1c3ef7ce9f7..595d7ed94bd 100644
--- a/src/metabase/driver/mongo/util.clj
+++ b/src/metabase/driver/mongo/util.clj
@@ -90,12 +90,11 @@
;; nil)
(take 1000)
(group-by type)
- ;; create tuples like [Integer -count]. Make count negative so when we call (sort-by second) in the next step the rows
- ;; with the highest count will be returned first (e.g. [Integer -100] will be sorted ahead of [Float -20])
+ ;; create tuples like [Integer count].
(map (fn [[klass valus]]
- [klass (- 0 (count valus))]))
+ [klass (count valus)]))
(sort-by second)
- first
- first
- driver/class->base-type)
+ last ; last result will be tuple with highest count
+ first ; keep just the type
+ driver/class->base-type) ; convert to Field base_type
:UnknownField))
diff --git a/src/metabase/driver/sync.clj b/src/metabase/driver/sync.clj
index 8b142b48f3d..311a4f0c4b4 100644
--- a/src/metabase/driver/sync.clj
+++ b/src/metabase/driver/sync.clj
@@ -151,8 +151,8 @@
(let [database @(:db table)]
;; Now do the syncing for Table's Fields
(log/debug (format "Determining active Fields for Table '%s'..." (:name table)))
- (let [active-column-names->type (active-column-names->type driver table)
- existing-field-name->id (sel :many :field->id [Field :name], :table_id (:id table), :active true, :parent_id nil)]
+ (let [active-column-names->type (active-column-names->type driver table)
+ existing-field-name->field (sel :many :field->fields [Field :name :base_type :id], :table_id (:id table), :active true, :parent_id nil)]
(assert (map? active-column-names->type) "active-column-names->type should return a map.")
(assert (every? string? (keys active-column-names->type)) "The keys of active-column-names->type should be strings.")
@@ -160,20 +160,27 @@
;; As above, first mark inactive Fields
(let [active-column-names (set (keys active-column-names->type))]
- (doseq [[field-name field-id] existing-field-name->id]
+ (doseq [[field-name {field-id :id}] existing-field-name->field]
(when-not (contains? active-column-names field-name)
(upd Field field-id :active false)
(log/info (u/format-color 'cyan "Marked field '%s.%s' as inactive." (:name table) field-name)))))
- ;; Next, create new Fields as needed
- (let [existing-field-names (set (keys existing-field-name->id))]
+ ;; Create new Fields, update existing types if needed
+ (let [existing-field-names (set (keys existing-field-name->field))]
(doseq [[active-field-name active-field-type] active-column-names->type]
- (when-not (contains? existing-field-names active-field-name)
- (log/info (u/format-color 'blue "Found new field: '%s.%s'" (:name table) active-field-name))
- (ins Field
- :table_id (:id table)
- :name active-field-name
- :base_type active-field-type))))
+ ;; If Field doesn't exist create it
+ (if-not (contains? existing-field-names active-field-name)
+ (do (log/info (u/format-color 'blue "Found new field: '%s.%s'" (:name table) active-field-name))
+ (ins Field
+ :table_id (:id table)
+ :name active-field-name
+ :base_type active-field-type))
+ ;; Otherwise update the Field type if needed
+ (let [{existing-base-type :base_type, existing-field-id :id} (existing-field-name->field active-field-name)]
+ (when-not (= active-field-type existing-base-type)
+ (log/info (u/format-color 'blue "Field '%s.%s' has changed from a %s to a %s." (:name table) active-field-name existing-base-type active-field-type))
+ (upd Field existing-field-id :base_type active-field-type))))))
+ ;; TODO - we need to add functionality to update nested Field base types as well!
;; Now mark PK fields as such if needed
(let [pk-fields (table-pks driver table)]
--
GitLab