diff --git a/src/metabase/driver/mongo.clj b/src/metabase/driver/mongo.clj
index 4d2e4825510406b9a4660a3c5232918e25546c55..30d5f4cd469fe9f4e5e72ae106e5803da573dbbe 100644
--- a/src/metabase/driver/mongo.clj
+++ b/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 ? )
diff --git a/src/metabase/driver/mongo/query_processor.clj b/src/metabase/driver/mongo/query_processor.clj
index c3dd26f84135084d23512a2fb969f0b75e9709b9..f5704da56eb82f48374a9437b919ef2f3cc3dafe 100644
--- a/src/metabase/driver/mongo/query_processor.clj
+++ b/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
diff --git a/src/metabase/driver/mongo/util.clj b/src/metabase/driver/mongo/util.clj
index 142a950efabf407fc3cfd8e86bc3697b0000f6ad..07673fcb539c32ec7d23d91e163b8aacf37ac6e3 100644
--- a/src/metabase/driver/mongo/util.clj
+++ b/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)
diff --git a/src/metabase/driver/query_processor.clj b/src/metabase/driver/query_processor.clj
index 683dd912f8e07fd7ced34ddebeba758e3fb601f8..3ef593da64a0c20301383b5a2444e0f813e06023 100644
--- a/src/metabase/driver/query_processor.clj
+++ b/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)))
diff --git a/src/metabase/models/field.clj b/src/metabase/models/field.clj
index 1ea30104044ec13bc4976c85855989adcac62b5f..deae78079c2660c16744ba986cd1b4f65d2d9400 100644
--- a/src/metabase/models/field.clj
+++ b/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
diff --git a/test/metabase/driver/query_processor_test.clj b/test/metabase/driver/query_processor_test.clj
index a317e2bb76cb32ae15bb3bc1ac6f277f06045f3e..06bd5d2f58ed54de794face1d44ce88442277437 100644
--- a/test/metabase/driver/query_processor_test.clj
+++ b/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]
diff --git a/test/metabase/test/data.clj b/test/metabase/test/data.clj
index 350df2cbada97d06338f7a5bccbb3a5bd4f57b04..00502baecaf9524e4df9ace39e3cc328522087a8 100644
--- a/test/metabase/test/data.clj
+++ b/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.
diff --git a/test/metabase/test/util/mql.clj b/test/metabase/test/util/mql.clj
index 361177ebeb5e82120e3913b662afc029ffb86e44..de36eb7a1bbed5f5f65fa0e6c29bb946ba8e5418 100644
--- a/test/metabase/test/util/mql.clj
+++ b/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