write some testz

src/metabase/driver/mongo.clj

@@ -87,15 +87,18 @@
     #{"_id"})
 
   ISyncDriverFieldValues
-  (field-values-lazy-seq [_ field]
+  (field-values-lazy-seq [_ {:keys [qualified-name-components table], :as field}]
     {:pre [(map? field)
-           (delay? (:table field))]}
+           (delay? qualified-name-components)
+           (delay? table)]}
     (lazy-seq
-     (let [table @(:table field)]
-       (map (keyword (:name field))
-            (with-mongo-connection [^com.mongodb.DBApiLayer conn @(:db table)]
-              (mq/with-collection conn (:name table)
-                (mq/fields [(:name field)])))))))
+     (assert *mongo-connection*
+             "You must have an open Mongo connection in order to get lazy results with field-values-lazy-seq.")
+     (let [table           @table
+           name-components (rest @qualified-name-components)]
+       (map #(get-in % (map keyword name-components))
+            (mq/with-collection *mongo-connection* (:name table)
+              (mq/fields [(apply str (interpose "." name-components))]))))))
 
   ISyncDriverFieldNestedFields
   (active-nested-field-name->type [this field]
@@ -120,44 +123,3 @@
 (def driver
   "Concrete instance of the MongoDB driver."
   (MongoDriver.))
-
-
-
-;; ---------------------------------------- EXPLORATORY SUBFIELD STUFF ----------------------------------------
-
-(require '[metabase.test.data :as data]
-         '[metabase.test.data.datasets :as datasets]
-         '[metabase.test.data.dataset-definitions :as defs]
-         '[metabase.test.util.mql :refer [Q]]
-         '[metabase.db :refer [sel]]
-         '(metabase.models [table :refer [Table]]
-                           [field :refer [Field]]))
-
-(defn x []
-  (Q run with db geographical-tips
-     with dataset mongo
-     ag rows
-     tbl tips
-     filter = venue...name "Kyle's Low-Carb Grill"
-     lim 10))
-
-(defn x2 []
-  (Q run against geographical-tips using mongo
-     aggregate rows of tips
-     filter = source...service "yelp"
-     order venue...name+
-     limit 10))
-
-(defn y []
-  (datasets/with-dataset :mongo
-    (data/with-temp-db [db (data/dataset-loader) defs/geographical-tips]
-      (->> (sel :many :fields [Field :id :name :parent_id] :table_id (sel :one :id Table :db_id (:id db)))
-           metabase.models.field/unflatten-nested-fields))))
-
-;; TODO
-;; 4. API
-;;    4A. API Tweaks as Needed
-;; 5. Cleanup + Tests
-;;    5A. Cleanup / Dox
-;;    5B. Tests
-;;    5C. $ notation doesn't handle nested Fields (yet) (or id ? )

src/metabase/driver/mongo/query_processor.clj

@@ -107,7 +107,7 @@
   ([field]
    `[{:count (mc/count ^DBApiLayer *mongo-connection* ~*collection-name*
                        (merge ~*constraints*
-                              {(field->name field) {$exists true}}))}]))
+                              {~(field->name field) {$exists true}}))}]))
 
 (defn- aggregation:avg [field]
   (aggregate {$group {"_id" nil

src/metabase/driver/mongo/util.clj

@@ -34,7 +34,7 @@
        "?connectTimeoutMS="
        connection-timeout-ms))
 
-(def ^:dynamic *mongo-connection*
+(def ^:dynamic ^com.mongodb.DBApiLayer *mongo-connection*
   "Connection to a Mongo database.
    Bound by top-level `with-mongo-connection` so it may be reused within its body."
   nil)

src/metabase/driver/query_processor.clj

@@ -278,6 +278,8 @@
                               (filter (complement (partial contains? (set (concat breakout-kws non-breakout-kws))))))]
 
     ;; Now combine the breakout [#1] + aggregate [#2] + "non-breakout" [#3 &  #4] column name keywords into a single sequence
+    (when-not *disable-qp-logging*
+      (log/debug (u/format-color 'magenta "Using this ordering: breakout: %s, ag: %s, other: %s" (vec breakout-kws) (vec ag-kws) (vec non-breakout-kws))))
     (concat breakout-kws ag-kws non-breakout-kws)))
 
 (defn- add-fields-extra-info
@@ -355,10 +357,14 @@
           results                        (if-not uncastify-fn results
                                                  (for [row results]
                                                    (m/map-keys uncastify-fn row)))
+          _                              (when-not *disable-qp-logging*
+                                           (log/debug (u/format-color 'magenta "\nDriver QP returned results with keys: %s." (vec (keys (first results))))))
           join-table-ids                 (set (map :table-id join-tables))
           fields                         (sel :many :fields [Field :id :table_id :name :description :base_type :special_type],
-                                              :table_id source-table-id, :active true)
+                                              :table_id source-table-id, :active true, :parent_id nil)
           ordered-col-kws                (order-cols query results fields)]
+      (assert (= (count (keys (first results))) (count ordered-col-kws))
+              (format "Order-cols returned an invalid number of keys. Expected: %d, got: %d" (count (keys (first results))) (count ordered-col-kws)))
       {:rows    (for [row results]
                   (mapv row ordered-col-kws))                                          ; might as well return each row and col info as vecs because we're not worried about making
        :columns (mapv name ordered-col-kws)                                            ; making them lazy, and results are easier to play with in the REPL / paste into unit tests
@@ -405,13 +411,6 @@
       (reset! called? true)
       (qp query))))
 
-(defn- post-log-results [qp]
-  (fn [query]
-    (let [results (qp query)]
-      (when-not *disable-qp-logging*
-        (log/debug "\nRESULTS:\n" (u/pprint-to-str 'cyan results)))
-      results)))
-
 (defn- process-structured [{:keys [driver], :as query}]
   (let [driver-process-query (partial i/process-query driver)]
     ((<<- wrap-catch-exceptions
@@ -424,7 +423,6 @@
           limit
           post-annotate
           pre-log-query
-          post-log-results
           wrap-guard-multiple-calls
           driver-process-query) query)))
 

src/metabase/models/field.clj

@@ -108,28 +108,29 @@
                                                          (parent-id->fields (:id field)))))]
      (map resolve-children (parent-id->fields nil)))))
 
-(defn- qualified-name
-  "Return a name like `table.field` for FIELD. If FIELD is a nested field, recursively return a name
-   like `table.parent.field`."
+(defn- qualified-name-components
+  "Return the pieces that represent a path to FIELD, of the form `[table-name parent-fields-name* field-name]`."
   [{:keys [table parent], :as field}]
   {:pre [(delay? table)]}
-  (str (if parent
-         (qualified-name @parent)
-         (:name @table))
-       "." (:name field)))
+  (conj (if parent
+          (qualified-name-components @parent)
+          [(:name @table)])
+        (:name field)))
 
 (defmethod post-select Field [_ {:keys [table_id] :as field}]
   (u/assoc* field
-    :table               (delay (sel :one 'metabase.models.table/Table :id table_id))
-    :db                  (delay @(:db @(:table <>)))
-    :target              (delay (field->fk-field field))
-    :can_read            (delay @(:can_read @(:table <>)))
-    :can_write           (delay @(:can_write @(:table <>)))
-    :human_readable_name (when (name :field)
-                           (delay (common/name->human-readable-name (:name field))))
-    :parent              (when (:parent_id field)
-                           (delay (sel :one Field :id (:parent_id field))))
-    :qualified-name      (delay (qualified-name <>))))
+    :table                     (delay (sel :one 'metabase.models.table/Table :id table_id))
+    :db                        (delay @(:db @(:table <>)))
+    :target                    (delay (field->fk-field field))
+    :can_read                  (delay @(:can_read @(:table <>)))
+    :can_write                 (delay @(:can_write @(:table <>)))
+    :human_readable_name       (when (name :field)
+                                 (delay (common/name->human-readable-name (:name field))))
+    :parent                    (when (:parent_id field)
+                                 (delay (sel :one Field :id (:parent_id field))))
+    :children                  (delay (sel :many Field :parent_id (:id field)))
+    :qualified-name-components (delay (qualified-name-components <>))
+    :qualified-name            (delay (apply str (interpose "." @(:qualified-name-components <>))))))
 
 (defmethod pre-insert Field [_ field]
   (let [defaults {:active          true

test/metabase/driver/query_processor_test.clj

@@ -975,6 +975,77 @@
 ;; |                                                MONGO NESTED-FIELD ACCESS                                               |
 ;; +------------------------------------------------------------------------------------------------------------------------+
 
+;;; Nested Field in FILTER
+;; Get the first 10 tips where tip.venue.name == "Kyle's Low-Carb Grill"
+(expect
+    [[8 "Kyle's Low-Carb Grill"]
+     [67 "Kyle's Low-Carb Grill"]
+     [80 "Kyle's Low-Carb Grill"]
+     [83 "Kyle's Low-Carb Grill"]
+     [295 "Kyle's Low-Carb Grill"]
+     [342 "Kyle's Low-Carb Grill"]
+     [417 "Kyle's Low-Carb Grill"]
+     [426 "Kyle's Low-Carb Grill"]
+     [470 "Kyle's Low-Carb Grill"]]
+  (Q run against geographical-tips using mongo
+     return :data :rows (map (fn [[id _ _ {venue-name :name}]] [id venue-name]))
+     aggregate rows of tips
+     filter = venue...name "Kyle's Low-Carb Grill"
+     order _id
+     lim 10))
+
+;;; Nested Field in ORDER
+;; Let's get all the tips Kyle posted on Twitter sorted by tip.venue.name
+(expect
+    [[446 "Cam's Mexican Gastro Pub"
+      {:mentions ["@cams_mexican_gastro_pub"], :tags ["#mexican" "#gastro" "#pub"], :service "twitter", :username "kyle"}]
+     [230 "Haight European Grill"
+      {:mentions ["@haight_european_grill"], :tags ["#european" "#grill"], :service "twitter", :username "kyle"}]
+     [319 "Haight Soul Food Pop-Up Food Stand"
+      {:mentions ["@haight_soul_food_pop_up_food_stand"], :tags ["#soul" "#food" "#pop-up" "#food" "#stand"], :service "twitter", :username "kyle"}]
+     [224 "Pacific Heights Free-Range Eatery"
+      {:mentions ["@pacific_heights_free_range_eatery"], :tags ["#free-range" "#eatery"], :service "twitter", :username "kyle"}]]
+  (Q run against geographical-tips using mongo
+     return :data :rows (map (fn [[id source _ {venue-name :name}]] [id venue-name source]))
+     aggregate rows of tips
+     filter and = source...service "twitter"
+     = source...username "kyle"
+     order venue...name))
+
+;; Nested Field in AGGREGATION
+(expect
+    {}
+    (Q run against geographical-tips using mongo
+       return :data :rows
+       aggregate distinct venue...name of tips))
+
+;;; Nested Field in BREAKOUT
+
+;; TODO - id/$ don't handle nested Fields ?
+
+;;; Nested Field in FIELDS
+;; Return the first 10 tips with just tip.venue.name
+(expect
+    [[1 {:name "Lucky's Gluten-Free Café"}]
+     [2 {:name "Joe's Homestyle Eatery"}]
+     [3 {:name "Lower Pac Heights Cage-Free Coffee House"}]
+     [4 {:name "Oakland European Liquor Store"}]
+     [5 {:name "Tenderloin Gormet Restaurant"}]
+     [6 {:name "Marina Modern Sushi"}]
+     [7 {:name "Sunset Homestyle Grill"}]
+     [8 {:name "Kyle's Low-Carb Grill"}]
+     [9 {:name "Mission Homestyle Churros"}]
+     [10 {:name "Sameer's Pizza Liquor Store"}]]
+  (Q run against geographical-tips using mongo
+     return :data :rows
+     aggregate rows of tips
+     order _id
+     fields venue...name
+     lim 10))
+
+
+;;; Nested-Nested Fields
+
 (defn y []
   (datasets/with-dataset :mongo
     (with-temp-db [_ (dataset-loader) defs/geographical-tips]

test/metabase/test/data.clj

@@ -178,19 +178,26 @@
   ([temp-db table-name]
    {:pre [(map? temp-db)
           (string? table-name)]
-    :post [(map? %)]}
+    :post [(or (map? %) (assert nil (format "Couldn't find table '%s'.\nValid choices are: %s" table-name
+                                            (vec (keys @(:table-name->table temp-db))))))]}
    (@(:table-name->table temp-db) table-name))
 
   ([temp-db table-name field-name]
    {:pre [(string? field-name)]
-    :post [(map? %)]}
+    :post [(or (map? %) (assert nil (format "Couldn't find field '%s.%s'.\nValid choices are: %s" table-name field-name
+                                            (vec (keys @(:field-name->field (-temp-get temp-db table-name)))))))]}
    (@(:field-name->field (-temp-get temp-db table-name)) field-name))
 
   ([temp-db table-name parent-field-name & nested-field-names]
    {:pre [(every? string? nested-field-names)]
-    :post [(map? %)]}
+    :post [(or (map? %) (assert nil (format "Couldn't find nested field '%s.%s.%s'.\nValid choices are: %s" table-name parent-field-name
+                                            (apply str (interpose "." nested-field-names))
+                                            (vec (map :name @(:children (apply -temp-get temp-db table-name parent-field-name (butlast nested-field-names))))))))]}
    (binding [*sel-disable-logging* true]
-     (sel :one Field, :name (last nested-field-names), :parent_id (:id (apply -temp-get temp-db table-name parent-field-name (butlast nested-field-names)))))))
+     (let [parent            (apply -temp-get temp-db table-name parent-field-name (butlast nested-field-names))
+           children          @(:children parent)
+           child-name->child (zipmap (map :name children) children)]
+       (child-name->child (last nested-field-names))))))
 
 (defn- walk-expand-&
   "Walk BODY looking for symbols like `&table` or `&table.field` and expand them to appropriate `-temp-get` forms.

test/metabase/test/util/mql.clj

@@ -57,7 +57,7 @@
                           ~query)))))
 
 (defmacro Q:return [q & args]
-  `(-> ~q ~@args))
+  `(->> ~q ~@args))
 
 (defmacro Q:expand-outer [token form]
   (macroexpand-all `(symbol-macrolet [~'against Q:against