diff --git a/.dir-locals.el b/.dir-locals.el
index bce0983be7cecf51c44bff345095c2a299cfa0e2..3120b81a99d175b6bfa914d69b80397f8ef9e61b 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -36,6 +36,7 @@
(org-perms-case 1)
(pdoseq 1)
(qp-expect-with-datasets 1)
+ (query-with-temp-db 1)
(resolve-private-fns 1)
(symbol-macrolet 1)
(sync-in-context 2)
diff --git a/src/metabase/driver/query_processor.clj b/src/metabase/driver/query_processor.clj
index 1c9952c064ffc7b0f3fc7ea468d8ba2b2086d8df..768fc7eadf2df0859d6dab4918a32d9c4fe9dba6 100644
--- a/src/metabase/driver/query_processor.clj
+++ b/src/metabase/driver/query_processor.clj
@@ -407,7 +407,7 @@
(defn- get-cols-info
"Get column info for the `:cols` part of the QP results."
- [{{{ag-type :aggregation-type, ag-field :field} :aggregation} :query} fields ordered-col-kws]
+ [{{{ag-type :aggregation-type, ag-field :field} :aggregation} :query} fields ordered-col-kws join-table-ids]
(let [field-kw->field (zipmap (map #(keyword (:name %)) fields)
fields)
field-id->field (delay (zipmap (map :id fields) ; a delay since we probably won't need it
@@ -416,6 +416,11 @@
(or
;; If col-kw is a known Field return that
(field-kw->field col-kw)
+
+ ;; Otherwise if this Query included any joins then attempt to lookup a matching Field from one of the join tables
+ (and (seq join-table-ids)
+ (sel :one :fields [Field :id :table_id :name :description :base_type :special_type], :name (name col-kw), :table_id [in join-table-ids]))
+
;; Otherwise it is an aggregation column like :sum, build a map of information to return
(merge (assert ag-type)
{:name (name col-kw)
@@ -444,10 +449,10 @@
(let [results (if-not uncastify-fn results
(for [row results]
(m/map-keys uncastify-fn row)))
- table-ids (set (conj (map :table-id join-tables) source-table-id))
- fields (sel :many :fields [Field :id :table_id :name :description :base_type :special_type], :table_id [in table-ids], :active true)
+ 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)
ordered-col-kws (order-cols query results fields)]
{: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
- :cols (vec (get-cols-info query fields ordered-col-kws))})) ; as vecs. Make sure
+ (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
+ :cols (vec (get-cols-info query fields ordered-col-kws join-table-ids))})) ; as vecs.
diff --git a/test/metabase/driver/query_processor_test.clj b/test/metabase/driver/query_processor_test.clj
index 42e8b1c8e1d338931c1e2aacd7b777675493b83b..212462acd21ebbbcb1f5efbf6631ec37f82679a6 100644
--- a/test/metabase/driver/query_processor_test.clj
+++ b/test/metabase/driver/query_processor_test.clj
@@ -760,22 +760,29 @@
(update-in [:data :rows] (partial mapv (partial filterv #(not (isa? (type %) java.util.Date)))))))
-;; ## Unix timestamp special type fields <3
+;; +------------------------------------------------------------------------------------------------------------------------+
+;; | UNIX TIMESTAMP SPECIAL_TYPE FIELDS |
+;; +------------------------------------------------------------------------------------------------------------------------+
+
+(defmacro ^:private query-with-temp-db
+ "Convenience to generate a `with-temp-db` wrapping a `driver/process-query` form.
+ See usage below."
+ [defs & {:as query}]
+ `(with-temp-db [db# (dataset-loader) ~defs]
+ (driver/process-query {:database (:id db#)
+ :type :query
+ :query ~query})))
;; There were 9 "sad toucan incidents" on 2015-06-02
(datasets/expect-with-datasets #{:generic-sql}
9
- (with-temp-db [db (dataset-loader) defs/sad-toucan-incidents]
- (->> (driver/process-query {:database (:id db)
- :type "query"
- :query {:source_table (:id &incidents)
- :filter ["AND"
- [">" (:id &incidents.timestamp) "2015-06-01"]
- ["<" (:id &incidents.timestamp) "2015-06-03"]]
- :order_by [[(:id &incidents.timestamp) "ascending"]]}})
- :data
- :rows
- count)))
+ (->> (query-with-temp-db defs/sad-toucan-incidents
+ :source_table &incidents:id
+ :filter ["AND"
+ [">" &incidents.timestamp:id "2015-06-01"]
+ ["<" &incidents.timestamp:id "2015-06-03"]]
+ :order_by [[&incidents.timestamp:id "ascending"]])
+ :data :rows count))
;;; Unix timestamp breakouts -- SQL only
@@ -790,25 +797,22 @@
["2015-06-08" 9]
["2015-06-09" 7]
["2015-06-10" 8]]
- (with-temp-db [db (dataset-loader) defs/sad-toucan-incidents]
- (->> (driver/process-query {:database (:id db)
- :type "query"
- :query {:source_table (:id &incidents)
- :aggregation ["count"]
- :breakout [(:id &incidents.timestamp)]
- :limit 10}})
- :data
- :rows
- (map (fn [[^java.util.Date date count]]
- [(.toString date) (int count)])))))
+ (->> (query-with-temp-db defs/sad-toucan-incidents
+ :source_table &incidents:id
+ :aggregation ["count"]
+ :breakout [&incidents.timestamp:id]
+ :limit 10)
+ :data :rows
+ (map (fn [[^java.util.Date date count]]
+ [(.toString date) (int count)]))))
;; +------------------------------------------------------------------------------------------------------------------------+
-;; | JOIN |
+;; | JOINS |
;; +------------------------------------------------------------------------------------------------------------------------+
-;;; The top 10 cities by number of Tupac sightings
-;;; (test that we can breakout on an FK field)
+;; The top 10 cities by number of Tupac sightings
+;; Test that we can breakout on an FK field
(datasets/expect-with-datasets #{:generic-sql}
[["Arlington" 16]
["Albany" 15]
@@ -820,12 +824,70 @@
["Houston" 11]
["Irvine" 11]
["Lakeland" 11]]
- (with-temp-db [db (dataset-loader) defs/tupac-sightings]
- (-> (driver/process-query {:database (:id db)
- :type "query"
- :query {:source_table (:id &sightings)
- :aggregation ["count"]
- :breakout [(:id &cities.name)]
- :order_by [[["aggregation" 0] "descending"]]
- :limit 10}})
- :data :rows)))
+ (-> (query-with-temp-db defs/tupac-sightings
+ :source_table &sightings:id
+ :aggregation ["count"]
+ :breakout [&cities.name:id]
+ :order_by [[["aggregation" 0] "descending"]]
+ :limit 10)
+ :data :rows))
+
+
+;; Number of Tupac sightings in the Expa office
+;; (he was spotted here 60 times)
+;; Test that we can filter on an FK field
+(datasets/expect-with-datasets #{:generic-sql}
+ 60
+ (-> (query-with-temp-db defs/tupac-sightings
+ :source_table &sightings:id
+ :aggregation ["count"]
+ :filter ["=" (:id &categories.id) 8])
+ :data :rows first first))
+
+
+;; THE 10 MOST RECENT TUPAC SIGHTINGS (!)
+;; (What he was doing when we saw him, sighting ID)
+;; Check that we can include an FK field in the :fields clause
+(datasets/expect-with-datasets #{:generic-sql}
+ [["In the Park" 772]
+ ["Working at a Pet Store" 894]
+ ["At the Airport" 684]
+ ["At a Restaurant" 199]
+ ["Working as a Limo Driver" 33]
+ ["At Starbucks" 902]
+ ["On TV" 927]
+ ["At a Restaurant" 996]
+ ["Wearing a Biggie Shirt" 897]
+ ["In the Expa Office" 499]]
+ (->> (query-with-temp-db defs/tupac-sightings
+ :source_table &sightings:id
+ :fields [&sightings.id:id &categories.name:id]
+ :order_by [[&sightings.timestamp:id "descending"]]
+ :limit 10)
+ :data :rows))
+
+
+;; 1. Check that we can order by Foreign Keys
+;; (this query targets sightings and orders by cities.name and categories.name)
+;; 2. Check that we can join MULTIPLE tables in a single query
+;; (this query joins both cities and categories)
+(datasets/expect-with-datasets #{:generic-sql}
+ ;; CITY_ID, CATEGORY_ID, ID
+ ;; Cities are already alphabetized in the source data which is why CITY_ID is sorted
+ [[1 12 6]
+ [1 11 355]
+ [1 11 596]
+ [1 13 379]
+ [1 5 413]
+ [1 1 426]
+ [2 11 67]
+ [2 11 524]
+ [2 13 77]
+ [2 13 202]]
+ (->> (query-with-temp-db defs/tupac-sightings
+ :source_table &sightings:id
+ :order_by [[&cities.name:id "ascending"]
+ [&categories.name:id "descending"]
+ [&sightings.id:id "ascending"]]
+ :limit 10)
+ :data :rows (map butlast) (map reverse))) ; drop timestamps. reverse ordering to make the results columns order match order_by
diff --git a/test/metabase/test/data.clj b/test/metabase/test/data.clj
index 53bbb09657a494b09a1e2c86b1bbef9cf8ced63c..5991730eafebdb5792d02aec2e93a3e42533e4de 100644
--- a/test/metabase/test/data.clj
+++ b/test/metabase/test/data.clj
@@ -181,15 +181,20 @@
(@(:field-name->field (-temp-get temp-db table-name)) field-name)))
(defn- walk-expand-&
- "Walk BODY looking for symbols like `&table` or `&table.field` and expand them to appropriate `-temp-get` forms."
+ "Walk BODY looking for symbols like `&table` or `&table.field` and expand them to appropriate `-temp-get` forms.
+ If symbol ends in a `:field` form, wrap the call to `-temp-get` in call in a keyword getter for that field.
+
+ &sightings -> (-temp-get db \"sightings\")
+ &cities.name -> (-temp-get db \"cities\" \"name\")
+ &cities.name:id -> (:id (-temp-get db \"cities\" \"name\"))"
[db-binding body]
(walk/prewalk
(fn [form]
(or (when (symbol? form)
- (when-let [symbol-name (re-matches #"^&.+$" (name form))]
- `(-temp-get ~db-binding ~@(-> symbol-name
- (s/replace #"&" "")
- (s/split #"\.")))))
+ (when-let [[_ table-name field-name prop-name] (re-matches #"^&([^.:]+)(?:\.([^.:]+))?(?::([^.:]+))?$" (name form))]
+ (let [temp-get `(-temp-get ~db-binding ~table-name ~@(when field-name [field-name]))]
+ (if prop-name `(~(keyword prop-name) ~temp-get)
+ temp-get))))
form))
body))
@@ -199,8 +204,11 @@
Remove `Database` and destroy data afterward.
Within BODY, symbols like `&table` and `&table.field` will be expanded into function calls to
- fetch corresponding `Tables` and `Fields`. These are accessed via lazily-created maps of
- Table/Field names to the objects themselves. To facilitate mutli-driver tests, these names are lowercased.
+ fetch corresponding `Tables` and `Fields`. Symbols like `&table:id` wrap a getter around the resulting
+ forms (see `walk-expand-&` for details).
+
+ These are accessed via lazily-created maps of Table/Field names to the objects themselves.
+ To facilitate mutli-driver tests, these names are lowercased.
(with-temp-db [db (h2/dataset-loader) us-history-1607-to-1774]
(driver/process-quiery {:database (:id db)