diff --git a/src/metabase/query_processor.clj b/src/metabase/query_processor.clj
index 9e3b282ec55f58568108304f7b2fe455cef39692..0626c700ec1ce77fec5e1d65ce8d15896cad3418 100644
--- a/src/metabase/query_processor.clj
+++ b/src/metabase/query_processor.clj
@@ -7,7 +7,9 @@
[driver :as driver]]
[metabase.driver.util :as driver.u]
[metabase.mbql.schema :as mbql.s]
- [metabase.query-processor.debug :as debug]
+ [metabase.query-processor
+ [debug :as debug]
+ [store :as qp.store]]
[metabase.query-processor.middleware
[add-dimension-projections :as add-dim]
[add-implicit-clauses :as implicit-clauses]
@@ -241,6 +243,18 @@
preprocess
(m/dissoc-in [:middleware :disable-mbql->native?])))
+(defn query->expected-cols
+ "Return the `:cols` you would normally see in MBQL query results by preprocessing the query amd calling `annotate` on
+ it."
+ [{query-type :type, :as query}]
+ (when-not (= query-type :query)
+ (throw (Exception. (str (tru "Can only determine expected columns for MBQL queries.")))))
+ (let [results (qp.store/with-store
+ ((annotate/add-column-info (constantly nil))
+ (query->preprocessed query)))]
+ (or (seq (:cols results))
+ (throw (ex-info (str (tru "No columns returned.")) results)))))
+
(defn query->native
"Return the native form for `query` (e.g. for a MBQL query on Postgres this would return a map containing the compiled
SQL form). (Like `preprocess`, this function will throw an Exception if preprocessing was not successful.)
diff --git a/src/metabase/query_processor/middleware/add_implicit_clauses.clj b/src/metabase/query_processor/middleware/add_implicit_clauses.clj
index 8af8f86f310177fa1c1eb02cd1138d796ba5f85d..63e8ec05242e478d12a0ecf7e1ae370d94c87b4b 100644
--- a/src/metabase/query_processor/middleware/add_implicit_clauses.clj
+++ b/src/metabase/query_processor/middleware/add_implicit_clauses.clj
@@ -44,7 +44,7 @@
;; I suppose if we wanted to we could make the `order-by` rules swappable with something other set of rules
{:order-by (default-sort-rules)}))
-(s/defn sorted-implicit-fields-for-table :- [mbql.s/Field]
+(s/defn sorted-implicit-fields-for-table :- mbql.s/Fields
"For use when adding implicit Field IDs to a query. Return a sequence of field clauses, sorted by the rules listed
in `metabase.query-processor.sort`, for all the Fields in a given Table."
[table-id :- su/IntGreaterThanZero]
@@ -55,7 +55,7 @@
[:datetime-field [:field-id (u/get-id field)] :default]
[:field-id (u/get-id field)])))
-(s/defn ^:private source-metadata->fields :- [mbql.s/Field]
+(s/defn ^:private source-metadata->fields :- mbql.s/Fields
"Get implicit Fields for a query with a `:source-query` that has `source-metadata`."
[source-metadata :- (su/non-empty [mbql.s/SourceQueryMetadata])]
(for [{field-name :name, base-type :base_type} source-metadata]
diff --git a/src/metabase/query_processor/middleware/add_source_metadata.clj b/src/metabase/query_processor/middleware/add_source_metadata.clj
index 60fd485d3f4c47eaf5b6616f9818d3baf166ac88..c6915c9f6b6f9f0e3a1c54c82f526cdf131ece37 100644
--- a/src/metabase/query_processor/middleware/add_source_metadata.clj
+++ b/src/metabase/query_processor/middleware/add_source_metadata.clj
@@ -7,23 +7,9 @@
[metabase.query-processor
[interface :as qp.i]
[store :as qp.store]]
- [metabase.query-processor.middleware
- [add-implicit-clauses :as add-implicit-clauses]
- [annotate :as annotate]
- [pre-alias-aggregations :as pre-alias-ags]]
[metabase.util.i18n :refer [trs]]
[schema.core :as s]))
-(s/defn ^:private mbql-source-query->metadata :- [mbql.s/SourceQueryMetadata]
- [{breakouts :breakout
- aggregations :aggregation
- fields :fields
- :as source-query} :- mbql.s/SourceQuery]
- (let [field-ids (mbql.u/match (concat breakouts aggregations fields) [:field-id id] id)]
- (qp.store/fetch-and-store-fields! field-ids))
- (for [col (annotate/cols-for-mbql-query source-query)]
- (select-keys col [:name :id :table_id :display_name :base_type :special_type :unit :fingerprint :settings])))
-
(defn- has-same-fields-as-nested-source?
"Whether this source query itself has a nested source query, and will have the exact same fields in the results as its
nested source. If this is the case, we can return the `source-metadata` for the nested source as-is, if it is
@@ -39,39 +25,17 @@
(and (= (count fields) (count nested-source-metadata))
(every? #(mbql.u/is-clause? :field-literal (mbql.u/unwrap-field-clause %)) fields))))))
-(defn- can-determine-fields?
- "Whether we can determine the Fields that will come back in the results because at least one of `:breakout`,
- `:aggregation`, and/or `:fields` is present.
-
- When one or more of these is present, the QP will always return `breakouts + aggreagtions + fields`; if none are
- present, QP implementations fall back to the equivalent of `SELECT *`. Because we're calling
- `add-implicit-clauses/add-implicit-mbql-clauses` on the source query below, `:fields` should get added automatically
- to any MBQL source query or to any native source query with source metadata. Thus this should be true for every case
- except for native source queries with no source metadata, e.g.
-
- {:source-query {:native \"SELECT *\"}}"
- [{breakouts :breakout, aggregations :aggregation, fields :fields}]
- (some seq [breakouts aggregations fields]))
-
-(s/defn ^:private source-query->metadata :- (s/maybe [mbql.s/SourceQueryMetadata])
+(s/defn ^:private native-source-query->metadata :- (s/maybe [mbql.s/SourceQueryMetadata])
"Given a `source-query`, return the source metadata that should be added at the parent level (i.e., at the same
level where this `source-query` was present.) This metadata is used by other middleware to determine what Fields to
expect from the source query."
[{nested-source-metadata :source-metadata, :as source-query} :- mbql.s/SourceQuery]
- (cond
- ;; If the source query has a nested source with metadata and does not change the fields that come back, return
- ;; metadata as-is
- (has-same-fields-as-nested-source? source-query)
+ ;; If the source query has a nested source with metadata and does not change the fields that come back, return
+ ;; metadata as-is
+ (if (has-same-fields-as-nested-source? source-query)
nested-source-metadata
- ;;
- ;; otherwise if this query has at least one of `:breakout`, `:aggregation`, or `:fields`, the results are
- ;; determinate and we can generate appropriate metadata about the Fields that we can expect in the results
- (can-determine-fields? source-query)
- (mbql-source-query->metadata source-query)
- ;;
- ;; Otherwise we cannot determine the metadata automatically; usually, this is because the source query itself
- ;; has a native source query
- :else
+ ;; Otherwise we cannot determine the metadata automatically; usually, this is because the source query itself has
+ ;; a native source query
(do
(when-not qp.i/*disable-qp-logging*
(log/warn
@@ -79,23 +43,20 @@
{:source-query source-query}))
nil)))
-(defn- partially-preprocess-source-query
- "Unfortunately there are a few middleware transformations that would normally come later but need to be done here for
- nested source queries before proceeding. This middleware applies those transformations.
-
- TODO - in a nicer world there would be no need to do such things. It defeats the purpose of having distinct
- middleware stages."
+(s/defn ^:private mbql-source-query->metadata :- [mbql.s/SourceQueryMetadata]
+ "Preprocess a `source-query` so we can determine the result columns."
[source-query]
- (-> source-query
- pre-alias-ags/pre-alias-aggregations-in-inner-query
- add-implicit-clauses/add-implicit-mbql-clauses))
+ (let [cols ((resolve 'metabase.query-processor/query->expected-cols) {:database (:id (qp.store/database))
+ :type :query
+ :query source-query})]
+ (for [col cols]
+ (select-keys col [:name :id :table_id :display_name :base_type :special_type :unit :fingerprint :settings]))))
(s/defn ^:private add-source-metadata :- {:source-metadata [mbql.s/SourceQueryMetadata], s/Keyword s/Any}
[{{native-source-query? :native, :as source-query} :source-query, :as inner-query}]
- (let [source-query (if native-source-query?
- source-query
- (partially-preprocess-source-query source-query))
- metadata (source-query->metadata source-query)]
+ (let [metadata ((if native-source-query?
+ native-source-query->metadata
+ mbql-source-query->metadata) source-query)]
(assoc inner-query :source-metadata metadata)))
(defn- can-add-source-metadata?
@@ -112,12 +73,13 @@
(or (not native-source-query?)
source-query-has-source-metadata?)))
+(defn- maybe-add-source-metadata [x]
+ (if (and (map? x) (can-add-source-metadata? x))
+ (add-source-metadata x)
+ x))
+
(defn- add-source-metadata-at-all-levels [inner-query]
- (walk/postwalk
- #(if-not ((every-pred map? can-add-source-metadata?) %)
- %
- (add-source-metadata %))
- inner-query))
+ (walk/postwalk maybe-add-source-metadata inner-query))
(defn- add-source-metadata-for-source-queries* [{query-type :type, :as query}]
(if-not (= query-type :query)
diff --git a/src/metabase/query_processor/middleware/binning.clj b/src/metabase/query_processor/middleware/binning.clj
index a0ac8013dc00eb3d660a632d50b3c5ff3d55b464..fea69126f771d7dd430bd07b86de63e8507dfe87 100644
--- a/src/metabase/query_processor/middleware/binning.clj
+++ b/src/metabase/query_processor/middleware/binning.clj
@@ -199,17 +199,22 @@
[:binning-strategy field-clause new-strategy strategy-param (or (nicer-breakout new-strategy resolved-options)
resolved-options)]))
+(defn update-binning-strategy-in-inner-query
+ "Update `:binning-strategy` clauses in an `inner` [MBQL] query."
+ [{filters :filter, :as inner-query}]
+ (let [field-id->filters (filter->field-map filters)]
+ (mbql.u/replace inner-query
+ :binning-strategy
+ (try
+ (update-binned-field inner-query field-id->filters &match)
+ (catch Throwable e
+ (throw (ex-info (.getMessage e) {:clause &match} e)))))))
+
(defn- update-binning-strategy* [{query-type :type, inner-query :query, :as query}]
(if (= query-type :native)
query
- (let [field-id->filters (filter->field-map (get-in query [:query :filter]))]
- (mbql.u/replace-in query [:query]
- :binning-strategy
- (try
- (update-binned-field inner-query field-id->filters &match)
- (catch Throwable e
- (throw (ex-info (.getMessage e) {:clause &match} e))))))))
+ (update query :query update-binning-strategy-in-inner-query)))
(defn update-binning-strategy
"When a binned field is found, it might need to be updated if a relevant query criteria affects the min/max value of
diff --git a/src/metabase/query_processor/store.clj b/src/metabase/query_processor/store.clj
index 93ee2bc0e050f409a1f9b66eb8f823de0319589c..ac3fa90bec8313e6f771047da1b961e2bde31241 100644
--- a/src/metabase/query_processor/store.clj
+++ b/src/metabase/query_processor/store.clj
@@ -25,27 +25,32 @@
;;; ---------------------------------------------- Setting up the Store ----------------------------------------------
+(def ^:private uninitialized-store
+ (delay (throw (Exception. (str (tru "Error: Query Processor store is not initialized."))))))
+
(def ^:private ^:dynamic *store*
"Dynamic var used as the QP store for a given query execution."
- (delay (throw (Exception. (str (tru "Error: Query Processor store is not initialized."))))))
+ uninitialized-store)
(defn initialized?
"Is the QP store currently initialized?"
[]
- (not (delay? *store*)))
+ (not (identical? *store* uninitialized-store)))
-(defn do-with-new-store
- "Execute `f` with a freshly-bound `*store*`."
+(defn do-with-store
+ "Execute `f` with an initialized `*store*` if one is not already bound."
[f]
- (binding [*store* (atom {})]
- (f)))
+ (if (initialized?)
+ (f)
+ (binding [*store* (atom {})]
+ (f))))
(defmacro with-store
- "Execute `body` with a freshly-bound QP `*store*`. The `store` middleware takes care of setting up a fresh store for
- each query execution; you should have no need to use this macro yourself outside of that namespace."
+ "Execute `body` with an initialized QP `*store*`. The `store` middleware takes care of setting up a store as needed
+ for each query execution; you should have no need to use this macro yourself outside of that namespace."
{:style/indent 0}
[& body]
- `(do-with-new-store (fn [] ~@body)))
+ `(do-with-store (fn [] ~@body)))
(def ^:private database-columns-to-fetch
"Columns you should fetch for the Database referenced by the query before stashing in the store."
diff --git a/test/metabase/query_processor/middleware/add_source_metadata_test.clj b/test/metabase/query_processor/middleware/add_source_metadata_test.clj
index 00775833423677ff1557d42332d1e5b487402e47..9f89d73b737d6830030d18a094e4b660f057607b 100644
--- a/test/metabase/query_processor/middleware/add_source_metadata_test.clj
+++ b/test/metabase/query_processor/middleware/add_source_metadata_test.clj
@@ -7,7 +7,9 @@
[store :as qp.store]
[test-util :as qp.test-util]]
[metabase.query-processor.middleware.add-source-metadata :as add-source-metadata]
- [metabase.test.data :as data]))
+ [metabase.test
+ [data :as data]
+ [util :as tu]]))
(defn- add-source-metadata [query]
(driver/with-driver :h2
@@ -29,6 +31,22 @@
(for [field-name field-names]
(venues-metadata (keyword (str/lower-case (name field-name)))))))
+(defn- metadata-from-source-query
+ "`:fingerprint` and `:settings` are not usually included in `:source-metadata` if it is pulled up from a nested source
+ query, so this function can be used with one of the functions above to remove those keys if applicable.
+
+ TODO - I'm not convinced that behavior makes sense? Maybe we should change it and remove this function. "
+ [ms]
+ {:pre [(sequential? ms) (every? map? ms)]}
+ (for [m ms]
+ (reduce
+ (fn [m k]
+ (if (nil? (get m k))
+ (dissoc m k)
+ m))
+ m
+ [:fingerprint :settings])))
+
(defn- venues-source-metadata-for-field-literals
"Metadata we'd expect to see from a `:field-literal` clause. The same as normal metadata, but field literals don't
include special-type info."
@@ -50,7 +68,7 @@
;; Can we automatically add source metadata to the parent level of a query? If the source query does not have `:fields`
(expect
(data/mbql-query venues
- {:source-query {:source-table $$venues}
+ {:source-query {:source-table $$venues}
:source-metadata (venues-source-metadata)})
(add-source-metadata
(data/mbql-query venues
@@ -152,7 +170,7 @@
{:source-query {:source-query {:source-table $$venues
:fields [$id $name]}
:source-metadata (venues-source-metadata :id :name)}
- :source-metadata (venues-source-metadata :id :name)})
+ :source-metadata (metadata-from-source-query (venues-source-metadata :id :name))})
(add-source-metadata
(data/mbql-query venues
{:source-query {:source-query {:source-table $$venues
@@ -165,8 +183,8 @@
{:source-query {:source-query {:source-query {:source-table $$venues
:fields [$id $name]}
:source-metadata (venues-source-metadata :id :name)}
- :source-metadata (venues-source-metadata :id :name)}
- :source-metadata (venues-source-metadata :id :name)})
+ :source-metadata (metadata-from-source-query (venues-source-metadata :id :name))}
+ :source-metadata (metadata-from-source-query (venues-source-metadata :id :name))})
(add-source-metadata
(data/mbql-query venues
{:source-query
@@ -180,8 +198,8 @@
(data/mbql-query venues
{:source-query {:source-query {:source-query {:source-table $$venues}
:source-metadata (venues-source-metadata)}
- :source-metadata (venues-source-metadata)}
- :source-metadata (venues-source-metadata)})
+ :source-metadata (metadata-from-source-query (venues-source-metadata))}
+ :source-metadata (metadata-from-source-query (venues-source-metadata))})
(add-source-metadata
(data/mbql-query venues
{:source-query {:source-query {:source-query {:source-table $$venues}}}})))
@@ -192,8 +210,8 @@
{:source-query {:source-query {:source-query {:source-table $$venues
:fields [$id $name]}
:source-metadata (venues-source-metadata :id :name)}
- :source-metadata (venues-source-metadata :id :name)}
- :source-metadata (venues-source-metadata :id :name)})
+ :source-metadata (metadata-from-source-query (venues-source-metadata :id :name))}
+ :source-metadata (metadata-from-source-query (venues-source-metadata :id :name))})
(add-source-metadata
(data/mbql-query venues
{:source-query {:source-query {:source-query {:source-table $$venues
@@ -212,21 +230,21 @@
:aggregation [[:count]]
:breakout [$price]}
:source-metadata metadata}
- :source-metadata metadata}
- :source-metadata metadata}))
+ :source-metadata (metadata-from-source-query metadata)}
+ :source-metadata (metadata-from-source-query metadata)}))
(add-source-metadata
(data/mbql-query venues
{:source-query {:source-query {:source-query {:source-table $$venues
:aggregation [[:count]]
:breakout [$price]}}}})))
-;; can we add source-metadata to the parent level if the source query has a native source query, but has
+;; can we add `source-metadata` to the parent level if the source query has a native source query, but itself has
;; `source-metadata`?
(expect
(data/mbql-query venues
{:source-query {:source-query {:native "SELECT \"ID\", \"NAME\" FROM \"VENUES\";"}
:source-metadata (venues-source-metadata :id :name)}
- :source-metadata (venues-source-metadata :id :name)})
+ :source-metadata (metadata-from-source-query (venues-source-metadata :id :name))})
(add-source-metadata
(data/mbql-query venues
{:source-query {:source-query {:native "SELECT \"ID\", \"NAME\" FROM \"VENUES\";"}
@@ -244,3 +262,23 @@
{:source-table $$venues
:joins [{:source-query {:source-table $$venues
:fields [$id $name]}}]})))
+
+;; source metadata should handle source queries that have binned fields
+(expect
+ (data/mbql-query venues
+ {:source-query {:source-table $$venues
+ :aggregation [[:count]]
+ :breakout [[:binning-strategy $latitude :default]]}
+ :source-metadata (concat
+ (venues-source-metadata :latitude)
+ [{:name "count"
+ :display_name "count"
+ :base_type :type/Integer
+ :special_type :type/Number}])})
+ (tu/with-temporary-setting-values [breakout-bin-width 5.0]
+ (add-source-metadata
+ (data/mbql-query venues
+ {:source-query
+ {:source-table $$venues
+ :aggregation [[:count]]
+ :breakout [[:binning-strategy $latitude :default]]}}))))
diff --git a/test/metabase/query_processor/middleware/binning_test.clj b/test/metabase/query_processor/middleware/binning_test.clj
index a7f338f3193fec324fcae085693e18f7030117a0..dc4c7117b0d0c0108cb62852b4214b31e6a5600b 100644
--- a/test/metabase/query_processor/middleware/binning_test.clj
+++ b/test/metabase/query_processor/middleware/binning_test.clj
@@ -103,17 +103,20 @@
:num-bins 8}))
;; Try an end-to-end test of the middleware
-(tt/expect-with-temp [Field [field (field/map->FieldInstance
- {:database_type "DOUBLE"
- :table_id (data/id :checkins)
- :special_type :type/Income
- :name "TOTAL"
- :display_name "Total"
- :fingerprint {:global {:distinct-count 10000}
- :type {:type/Number {:min 12.061602936923117
- :max 238.32732001721533
- :avg 82.96014815230829}}}
- :base_type :type/Float})]]
+(defn- test-field []
+ (field/map->FieldInstance
+ {:database_type "DOUBLE"
+ :table_id (data/id :checkins)
+ :special_type :type/Income
+ :name "TOTAL"
+ :display_name "Total"
+ :fingerprint {:global {:distinct-count 10000}
+ :type {:type/Number {:min 12.061602936923117
+ :max 238.32732001721533
+ :avg 82.96014815230829}}}
+ :base_type :type/Float}))
+
+(tt/expect-with-temp [Field [field (test-field)]]
{:query {:source-table (data/id :checkins)
:breakout [[:binning-strategy
[:field-id (u/get-id field)]
@@ -128,3 +131,22 @@
:breakout [[:binning-strategy [:field-id (u/get-id field)] :default]]}
:type :query
:database (data/id)})))
+
+;; make sure `update-binning-strategy` works recursively on nested queries
+(tt/expect-with-temp [Field [field (test-field)]]
+ {:query {:source-query
+ {:source-table (data/id :checkins)
+ :breakout [[:binning-strategy
+ [:field-id (u/get-id field)]
+ :num-bins
+ nil
+ {:min-value 0.0, :max-value 240.0, :num-bins 8, :bin-width 30}]]}}
+ :type :query
+ :database (data/id)}
+ (qp.test-util/with-everything-store
+ ((binning/update-binning-strategy identity)
+ {:query {:source-query
+ {:source-table (data/id :checkins)
+ :breakout [[:binning-strategy [:field-id (u/get-id field)] :default]]}}
+ :type :query
+ :database (data/id)})))
diff --git a/test/metabase/query_processor_test.clj b/test/metabase/query_processor_test.clj
index 6cd400ef0517840817da07e8f7123e12f9c74226..0b82390494bff0e3dcd942111187d9daa6c8bd13 100644
--- a/test/metabase/query_processor_test.clj
+++ b/test/metabase/query_processor_test.clj
@@ -277,8 +277,9 @@
(try
(f v)
(catch Throwable e
- (printf "(%s %s) failed: %s" f v (.getMessage e))
- (throw e))))))))
+ (throw (ex-info (printf "format-rows-by failed (f = %s, value = %s %s): %s" f (.getName (class v)) v (.getMessage e))
+ {:f f, :v v}
+ e)))))))))
:else
(throw (ex-info "Unexpected response: rows are not sequential!" {:response response})))))))))
diff --git a/test/metabase/query_processor_test/breakout_test.clj b/test/metabase/query_processor_test/breakout_test.clj
index d71c591e58781afeaa14ef6f0379d26e606a00c3..8b136b64fa97f3388456f476e75c0bbcdb5d9beb 100644
--- a/test/metabase/query_processor_test/breakout_test.clj
+++ b/test/metabase/query_processor_test/breakout_test.clj
@@ -169,6 +169,17 @@
{:aggregation [[:count]]
:breakout [[:binning-strategy $latitude :default]]}))))
+;; Can I use `:default` binning in a nested query?
+(datasets/expect-with-drivers (qp.test/non-timeseries-drivers-with-feature :binning)
+ [[10.0 1] [30.0 61] [35.0 29] [40.0 9]]
+ (tu/with-temporary-setting-values [breakout-bin-width 5.0]
+ (qp.test/formatted-rows [1.0 int]
+ (data/run-mbql-query venues
+ {:source-query
+ {:source-table $$venues
+ :aggregation [[:count]]
+ :breakout [[:binning-strategy $latitude :default]]}}))))
+
;; Testing bin-width
(datasets/expect-with-drivers (qp.test/non-timeseries-drivers-with-feature :binning)
[[10.0 1] [33.0 4] [34.0 57] [37.0 29] [40.0 9]]
diff --git a/test/metabase/query_processor_test/explicit_joins_test.clj b/test/metabase/query_processor_test/explicit_joins_test.clj
index 56eb50044dbfb1e7c575b6951516ce2cb90861ed..cd907521e94ee5f22e3d1d602047a37035db714d 100644
--- a/test/metabase/query_processor_test/explicit_joins_test.clj
+++ b/test/metabase/query_processor_test/explicit_joins_test.clj
@@ -406,15 +406,34 @@
;;
;; (Multiple columns named `id` in the example below)
(datasets/expect-with-drivers (qp.test/non-timeseries-drivers-with-feature :left-join)
- {:rows
+ {:columns (mapv
+ data/format-name
+ ["id" "date" "user_id" "venue_id" ; checkins
+ "id_2" "name" "last_login" ; users
+ "id_2_2" "name_2" "category_id" "latitude" "longitude" "price"]) ; venues
+ :rows
;; again, not sure why Oracle is the only driver giving us wrong answers here
(let [tz (if (= driver/*driver* :oracle) "07:00:00.000Z" "00:00:00.000Z")]
- [[1 (str "2014-04-07T" tz) 5 12 5 "Brite Spot Family Restaurant" 20 34.078 -118.261 2]
- [2 (str "2014-09-18T" tz) 1 31 1 "Red Medicine" 4 10.065 -165.374 3]])
- :columns (mapv data/format-name
- ["id" "date" "user_id" "venue_id" "id_2" "name" "category_id" "latitude" "longitude" "price"])}
+ [[1 (str "2014-04-07T" tz) 5 12
+ 5 "Quentin Sören" "2014-10-03T17:30:00.000Z"
+ 5 "Brite Spot Family Restaurant" 20 34.078 -118.261 2]
+ [2 (str "2014-09-18T" tz) 1 31
+ 1 "Plato Yeshua" "2014-04-01T08:30:00.000Z"
+ 1 "Red Medicine" 4 10.065 -165.374 3]])}
(qp.test/rows+column-names
- (qp.test/format-rows-by [int str int int int str int 3.0 3.0 int]
+ (qp.test/format-rows-by [int ; checkins.id
+ str ; checkins.date
+ int ; checkins.user_id
+ int ; checkins.venue_id
+ int ; users.id
+ str ; users.name
+ str ; users.last_login
+ int ; venues.id
+ str ; venues.name
+ int ; venues.category_id
+ 3.0 ; venues.latitude
+ 3.0 ; venues.longitude
+ int] ; venues.price
(data/run-mbql-query checkins
{:source-query {:source-table $$checkins
:joins
@@ -428,15 +447,3 @@
:condition [:= *user_id &v.venues.id]}]
:order-by [[:asc $id]]
:limit 2}))))
-
-
-
-;; TODO Can we join against a source nested native query?
-
-;; TODO Do joins inside nested queries work?
-
-;; TODO Do multiple joins work, and return columns in the correct order?
-
-;; TODO Can we join the same table twice with different conditions?
-
-;; TODO Can we join the same table twice with the same condition?
diff --git a/test/metabase/query_processor_test/remapping_test.clj b/test/metabase/query_processor_test/remapping_test.clj
index 66674e1be7f0a34ab29105c538ea216248f82d10..f7c975842fbbabf6c12da30f8696bc308c908055 100644
--- a/test/metabase/query_processor_test/remapping_test.clj
+++ b/test/metabase/query_processor_test/remapping_test.clj
@@ -1,11 +1,9 @@
(ns metabase.query-processor-test.remapping-test
"Tests for the remapping results"
- (:require [metabase
- [query-processor :as qp]
- [query-processor-test :as qp.test]]
- [metabase.models
+ (:require [metabase.models
[dimension :refer [Dimension]]
[field :refer [Field]]]
+ [metabase.query-processor-test :as qp.test]
[metabase.query-processor.middleware.add-dimension-projections :as add-dimension-projections]
[metabase.test.data :as data]
[metabase.test.data.datasets :as datasets]
@@ -112,13 +110,9 @@
["20th Century Cafe" "25°" "33 Taps" "800 Degrees Neapolitan Pizzeria"]
(data/with-temp-objects
(data/create-venue-category-fk-remapping "Foo")
- (->> (qp/process-query
- {:database (data/id)
- :type :query
- :query {:source-table (data/id :venues)
- :order-by [[(data/id :venues :name) :ascending]]
- :limit 4}})
- qp.test/rows
+ (->> (qp.test/rows
+ (data/run-mbql-query venues
+ {:order-by [[:asc $name]], :limit 4}))
(map second))))
;; Test out a self referencing column. This has a users table like the one that is in `test-data`, but also includes a
@@ -132,9 +126,9 @@
(data/dataset test-data-self-referencing-user
(data/with-temp-objects
(fn []
- [(db/insert! Dimension {:field_id (data/id :users :created_by)
- :name "created-by-mapping"
- :type :external
+ [(db/insert! Dimension {:field_id (data/id :users :created_by)
+ :name "created-by-mapping"
+ :type :external
:human_readable_field_id (data/id :users :name)})])
(db/update! 'Field (data/id :users :created_by)