Everything is finally working

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

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\"},

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]

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
 

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."

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."

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))}

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))

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)]