From 034345550f6e7fd33d4791abac8148305014f510 Mon Sep 17 00:00:00 2001
From: Cam Saul <1455846+camsaul@users.noreply.github.com>
Date: Tue, 1 Feb 2022 00:23:28 -0800
Subject: [PATCH] Split QP middleware into different groups (#19935)
---
src/metabase/driver.clj | 11 +-
src/metabase/query_processor.clj | 170 ++-
.../middleware/add_rows_truncated.clj | 26 +-
.../middleware/forty_three.clj | 8 +-
.../query_processor/middleware/limit.clj | 4 +-
.../middleware/results_metadata.clj | 21 +-
src/metabase/query_processor/reducible.clj | 4 +-
src/metabase/query_processor/util.clj | 9 +-
.../middleware/add_rows_truncated_test.clj | 2 +-
.../query_processor/middleware/limit_test.clj | 3 +-
.../query_processor/util/nest_query_test.clj | 1212 ++++++++---------
.../query_processor_test/failure_test.clj | 7 +-
.../query_to_native_test.clj | 48 +-
13 files changed, 795 insertions(+), 730 deletions(-)
diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj
index 194574ee280..3dd3a943b09 100644
--- a/src/metabase/driver.clj
+++ b/src/metabase/driver.clj
@@ -491,13 +491,14 @@
(defmulti mbql->native
"Transpile an MBQL query into the appropriate native query form. `query` will match the schema for an MBQL query in
- `metabase.mbql.schema/Query`; this function should return a native query that conforms to that schema.
+ [[metabase.mbql.schema/Query]]; this function should return a native query that conforms to that schema.
- If the underlying query language supports remarks or comments, the driver should use `query->remark` to generate an
- appropriate message and include that in an appropriate place; alternatively a driver might directly include the
- query's `:info` dictionary if the underlying language is JSON-based.
+ If the underlying query language supports remarks or comments, the driver should
+ use [[metabase.query-processor.util/query->remark]] to generate an appropriate message and include that in an
+ appropriate place; alternatively a driver might directly include the query's `:info` dictionary if the underlying
+ language is JSON-based.
- The result of this function will be passed directly into calls to `execute-reducible-query`.
+ The result of this function will be passed directly into calls to [[execute-reducible-query]].
For example, a driver like Postgres would build a valid SQL expression and return a map such as:
diff --git a/src/metabase/query_processor.clj b/src/metabase/query_processor.clj
index 857ad7c1858..85ef335c85b 100644
--- a/src/metabase/query_processor.clj
+++ b/src/metabase/query_processor.clj
@@ -74,64 +74,124 @@
[column-level-perms-check :as ee.sandbox.columns]
[row-level-restrictions :as ee.sandbox.rows]]))
-;; ▼▼▼ POST-PROCESSING ▼▼▼ happens from TOP-TO-BOTTOM
-(def default-middleware
- "The default set of middleware applied to queries ran via `process-query`."
- [#'mbql-to-native/mbql->native ; EXECUTE
- #'check-features/check-features ; PRE
- #'limit/limit-result-rows-middleware ; POST
- #'limit/add-default-limit-middleware ; PRE
- #'cache/maybe-return-cached-results ; EXECUTE
- #'optimize-temporal-filters/optimize-temporal-filters ; PRE
- #'validate-temporal-bucketing/validate-temporal-bucketing ; PRE
- #'auto-parse-filter-values/auto-parse-filter-values ; PRE
- #'wrap-value-literals/wrap-value-literals ; PRE
- #'annotate/add-column-info ; POST
- #'perms/check-query-permissions ; EXECUTE
- #'pre-alias-ags/pre-alias-aggregations ; PRE
- #'cumulative-ags/sum-cumulative-aggregation-columns-middleware ; POST
- #'cumulative-ags/rewrite-cumulative-aggregations-middleware ; PRE
+(def ^:private pre-processing-middleware
+ "Pre-processing middleware. Has the form
+
+ (f query) -> query
+
+ in 43+."
+ [#'check-features/check-features
+ #'limit/add-default-limit-middleware
+ #'optimize-temporal-filters/optimize-temporal-filters
+ #'validate-temporal-bucketing/validate-temporal-bucketing
+ #'auto-parse-filter-values/auto-parse-filter-values
+ #'wrap-value-literals/wrap-value-literals
+ #'pre-alias-ags/pre-alias-aggregations
+ #'cumulative-ags/rewrite-cumulative-aggregations-middleware
;; yes, this is called a second time, because we need to handle any joins that got added
- (resolve 'ee.sandbox.rows/merge-sandboxing-metadata-middleware) ; POST
- (resolve 'ee.sandbox.rows/apply-sandboxing-middleware) ; PRE
- #'viz-settings/update-viz-settings ; POST
- #'fix-bad-refs/fix-bad-references-middleware ; PRE
- #'resolve-joined-fields/resolve-joined-fields ; PRE
- #'resolve-joins/resolve-joins ; PRE
- #'add-implicit-joins/add-implicit-joins ; PRE
- #'large-int-id/convert-id-to-string ; POST
- #'format-rows/format-rows ; POST
- #'add-default-temporal-unit/add-default-temporal-unit ; PRE
- #'desugar/desugar ; PRE
- #'binning/update-binning-strategy ; PRE
- #'resolve-fields/resolve-fields ; PRE
- #'add-dim/remap-results-middleware ; POST
- #'add-dim/add-remapped-columns-middleware ; PRE
- #'implicit-clauses/add-implicit-clauses ; PRE
- (resolve 'ee.sandbox.rows/merge-sandboxing-metadata-middleware) ; POST
- (resolve 'ee.sandbox.rows/apply-sandboxing-middleware) ; PRE
- #'upgrade-field-literals/upgrade-field-literals ; PRE
- #'add-source-metadata/add-source-metadata-for-source-queries ; PRE
- (resolve 'ee.sandbox.columns/maybe-apply-column-level-perms-check) ; EXECUTE
- #'reconcile-bucketing/reconcile-breakout-and-order-by-bucketing ; PRE
- #'bucket-datetime/auto-bucket-datetimes ; PRE
- #'resolve-source-table/resolve-source-tables ; PRE
- #'parameters/substitute-parameters ; PRE
- #'resolve-referenced/resolve-referenced-card-resources ; PRE
- #'expand-macros/expand-macros ; PRE
- #'add-timezone-info/add-timezone-info ; POST
- #'splice-params-in-response/splice-params-in-response ; POST
- #'resolve-database-and-driver/resolve-database-and-driver ; AROUND
- #'fetch-source-query/resolve-card-id-source-tables ; PRE
- #'store/initialize-store ; AROUND
- #'validate/validate-query ; PRE
- #'perms/remove-permissions-key-middleware ; PRE
- #'normalize/normalize ; PRE
- #'add-rows-truncated/add-rows-truncated ; POST
- (resolve 'ee.audit/handle-internal-queries) ; AROUND
- #'results-metadata/record-and-return-metadata!]) ; EXECUTE
+ (resolve 'ee.sandbox.rows/apply-sandboxing-middleware)
+ #'fix-bad-refs/fix-bad-references-middleware
+ #'resolve-joined-fields/resolve-joined-fields
+ #'resolve-joins/resolve-joins
+ #'add-implicit-joins/add-implicit-joins
+ #'add-default-temporal-unit/add-default-temporal-unit
+ #'desugar/desugar
+ #'binning/update-binning-strategy
+ #'resolve-fields/resolve-fields
+ #'add-dim/add-remapped-columns-middleware
+ #'implicit-clauses/add-implicit-clauses
+ (resolve 'ee.sandbox.rows/apply-sandboxing-middleware)
+ #'upgrade-field-literals/upgrade-field-literals
+ #'add-source-metadata/add-source-metadata-for-source-queries
+ #'reconcile-bucketing/reconcile-breakout-and-order-by-bucketing
+ #'bucket-datetime/auto-bucket-datetimes
+ #'resolve-source-table/resolve-source-tables
+ #'parameters/substitute-parameters
+ #'resolve-referenced/resolve-referenced-card-resources
+ #'expand-macros/expand-macros
+ #'validate/validate-query
+ #'perms/remove-permissions-key-middleware])
;; ▲▲▲ PRE-PROCESSING ▲▲▲ happens from BOTTOM-TO-TOP
+(def ^:private compile-middleware
+ "Middleware for query compilation. Happens after pre-processing. Has the form
+
+ (f query) -> query
+
+ in 43+."
+ [#'mbql-to-native/mbql->native])
+
+(def ^:private execution-middleware
+ "Middleware that happens after compilation, AROUND query execution itself. Has the form
+
+ (f qp) -> qp
+
+ Where `qp` has the form
+
+ (f query rff context)"
+ ;; TODO -- limit SEEMS like it should be post-processing but it actually has to happen only if we don't return cached
+ ;; results. Otherwise things break. There's probably some way to fix this. e.g. maybe it doesn't do anything if the
+ ;; query has the `:cached?` key.
+ [#'limit/limit-result-rows-middleware
+ #'cache/maybe-return-cached-results
+ #'perms/check-query-permissions
+ (resolve 'ee.sandbox.columns/maybe-apply-column-level-perms-check)])
+
+(def ^:private post-processing-middleware
+ "Post-processing middleware that transforms results. Has the form
+
+ (f preprocessed-query rff) -> rff
+
+ Where `rff` has the form
+
+ (f metadata) -> rf"
+ ;; ▼▼▼ POST-PROCESSING ▼▼▼ happens from TOP-TO-BOTTOM
+ [#'annotate/add-column-info
+ #'cumulative-ags/sum-cumulative-aggregation-columns-middleware
+ #'viz-settings/update-viz-settings
+ #'large-int-id/convert-id-to-string
+ #'format-rows/format-rows
+ #'add-dim/remap-results-middleware
+ (resolve 'ee.sandbox.rows/merge-sandboxing-metadata-middleware)
+ #'add-timezone-info/add-timezone-info
+ #'splice-params-in-response/splice-params-in-response
+ #'add-rows-truncated/add-rows-truncated-middleware])
+
+(def ^:private around-middleware
+ "Middleware that goes AROUND *all* the other middleware (even for pre-processing only or compilation only). Has the
+ form
+
+ (f qp) -> qp
+
+ Where `qp` has the form
+
+ (f query rff context)"
+ [#'resolve-database-and-driver/resolve-database-and-driver
+ #'fetch-source-query/resolve-card-id-source-tables
+ #'store/initialize-store
+ ;; `normalize` has to be done at the very beginning or `resolve-card-id-source-tables` and the like might not work.
+ ;; It doesn't really need to be 'around' middleware tho.
+ #'normalize/normalize
+ (resolve 'ee.audit/handle-internal-queries)
+ ;; TODO -- I think this is actually supposed to be post-processing middleware? #idk¿?
+ #'results-metadata/record-and-return-metadata!])
+
+;; query -> preprocessed = around + pre-process
+;; query -> native = around + pre-process + compile
+;; query -> results = around + pre-process + compile + execute + post-process = default-middleware
+
+(def default-middleware
+ "The default set of middleware applied to queries ran via [[process-query]]."
+ (into
+ []
+ (comp cat (keep identity))
+ [execution-middleware ; → → execute → → ↓
+ compile-middleware ; ↑ compile ↓
+ post-processing-middleware ; ↑ ↓ post-process
+ pre-processing-middleware ; ↑ pre-process ↓
+ around-middleware])) ; ↑ query ↓ results
+
+
;; In REPL-based dev rebuild the QP every time it is called; this way we don't need to reload this namespace when
;; middleware is changed. Outside of dev only build the QP once for performance/locality
(defn- base-qp [middleware]
diff --git a/src/metabase/query_processor/middleware/add_rows_truncated.clj b/src/metabase/query_processor/middleware/add_rows_truncated.clj
index dda6d2419a6..049970ec2b0 100644
--- a/src/metabase/query_processor/middleware/add_rows_truncated.clj
+++ b/src/metabase/query_processor/middleware/add_rows_truncated.clj
@@ -1,10 +1,17 @@
(ns metabase.query-processor.middleware.add-rows-truncated
"Adds `:rows_truncated` to the query results if the results were truncated because of the query's constraints."
(:require [metabase.query-processor.interface :as i]
- [metabase.query-processor.util :as qputil]))
+ [metabase.query-processor.middleware.forty-three :as m.43]
+ [metabase.query-processor.middleware.limit :as limit]))
-(defn- results-limit [{{:keys [max-results max-results-bare-rows]} :constraints, :as query}]
- (or (when (qputil/query-without-aggregations-or-limits? query)
+(defn- results-limit
+ [{{:keys [max-results max-results-bare-rows]} :constraints
+ {aggregations :aggregation, :keys [limit page], ::limit/keys [original-limit]} :query
+ :as query}]
+ (or (when (and (or (not limit)
+ (= original-limit nil))
+ (not page)
+ (empty? aggregations))
max-results-bare-rows)
max-results
i/absolute-max-results))
@@ -26,13 +33,12 @@
(vswap! row-count inc)
(rf result row)))))
-(defn add-rows-truncated
+(defn- add-rows-truncated [query rff]
+ (fn add-rows-truncated-rff* [metadata]
+ (add-rows-truncated-xform (results-limit query) (rff metadata))))
+
+(def add-rows-truncated-middleware
"Add `:rows_truncated` to the result if the results were truncated because of the query's constraints. Only affects QP
results that are reduced to a map (e.g. the default reducing function; other reducing functions such as streaming to
a CSV are unaffected.)"
- [qp]
- (fn [query rff context]
- (qp query
- (fn [metadata]
- (add-rows-truncated-xform (results-limit query) (rff metadata)))
- context)))
+ (m.43/wrap-43-post-processing-middleware add-rows-truncated))
diff --git a/src/metabase/query_processor/middleware/forty_three.clj b/src/metabase/query_processor/middleware/forty_three.clj
index 95ec2fcf423..2e86530a72c 100644
--- a/src/metabase/query_processor/middleware/forty_three.clj
+++ b/src/metabase/query_processor/middleware/forty_three.clj
@@ -14,8 +14,8 @@
(f qp) -> (f query rff context)"
[f]
- (fn [qp]
- (fn [query rff context]
+ (fn wrap-43-pre-processing-middleware-qp* [qp]
+ (fn wrap-43-pre-processing-middleware-fn* [query rff context]
(qp (f query) rff context))))
(defn wrap-43-post-processing-middleware
@@ -31,6 +31,6 @@
(f qp) -> (f query rff context)"
[f]
- (fn [qp]
- (fn [query rff context]
+ (fn wrap-43-post-processing-middleware-qp* [qp]
+ (fn wrap-43-post-processing-middleware-fn* [query rff context]
(qp query (f query rff) context))))
diff --git a/src/metabase/query_processor/middleware/limit.clj b/src/metabase/query_processor/middleware/limit.clj
index 98f443015eb..1adacdcffff 100644
--- a/src/metabase/query_processor/middleware/limit.clj
+++ b/src/metabase/query_processor/middleware/limit.clj
@@ -8,11 +8,11 @@
;;;; Pre-processing
-(defn- add-limit [max-rows {query-type :type, :as query}]
+(defn- add-limit [max-rows {query-type :type, {original-limit :limit}, :query, :as query}]
(cond-> query
(and (= query-type :query)
(qputil/query-without-aggregations-or-limits? query))
- (assoc-in [:query :limit] max-rows)))
+ (update :query assoc :limit max-rows, ::original-limit original-limit)))
(defn determine-query-max-rows
"Given a `query`, return the max rows that should be returned. This is the first non-nil value from (in decreasing
diff --git a/src/metabase/query_processor/middleware/results_metadata.clj b/src/metabase/query_processor/middleware/results_metadata.clj
index 17d2f6f4ec1..bcdb87912c1 100644
--- a/src/metabase/query_processor/middleware/results_metadata.clj
+++ b/src/metabase/query_processor/middleware/results_metadata.clj
@@ -4,12 +4,12 @@
as a checksum in the API response."
(:require [clojure.tools.logging :as log]
[metabase.driver :as driver]
+ [metabase.query-processor.middleware.forty-three :as m.43]
[metabase.query-processor.reducible :as qp.reducible]
[metabase.sync.analyze.query-results :as analyze.results]
[metabase.util.i18n :refer [tru]]
[toucan.db :as db]))
-
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Middleware |
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -67,16 +67,15 @@
(rf (cond-> result
(map? result)
(update :data assoc
- :results_metadata {:columns metadata}
+ :results_metadata {:columns metadata}
:insights insights)))))))
-(defn record-and-return-metadata!
+(defn- record-and-return-metadata!*
+ [{{:keys [skip-results-metadata?]} :middleware, :as query} rff]
+ (let [record! (partial record-metadata! query)]
+ (fn record-and-return-metadata!-rff* [metadata]
+ (insights-xform metadata record! (rff metadata)))))
+
+(def record-and-return-metadata!
"Middleware that records metadata about the columns returned when running the query."
- [qp]
- (fn [{{:keys [skip-results-metadata?]} :middleware, :as query} rff context]
- (if skip-results-metadata?
- (qp query rff context)
- (let [record! (partial record-metadata! query)
- rff' (fn [metadata]
- (insights-xform metadata record! (rff metadata)))]
- (qp query rff' context)))))
+ (m.43/wrap-43-post-processing-middleware record-and-return-metadata!*))
diff --git a/src/metabase/query_processor/reducible.clj b/src/metabase/query_processor/reducible.clj
index 1c6a922dc3e..f7563d6e1f0 100644
--- a/src/metabase/query_processor/reducible.clj
+++ b/src/metabase/query_processor/reducible.clj
@@ -41,6 +41,8 @@
[middleware]
(reduce
(fn [qp middleware]
+ (when (var? middleware)
+ (assert (not (:private (meta middleware))) (format "%s is private" (pr-str middleware))))
(if (some? middleware)
(middleware qp)
qp))
@@ -194,7 +196,7 @@
{:pre [(fn? primary-rf) (sequential? additional-rfs) (every? fn? additional-rfs) (fn? combine)]}
(let [additional-accs (volatile! (mapv (fn [rf] (rf))
additional-rfs))]
- (fn
+ (fn combine-additional-reducing-fns-rf*
([] (primary-rf))
([acc]
diff --git a/src/metabase/query_processor/util.clj b/src/metabase/query_processor/util.clj
index 1a2400570ef..5f7cb24a141 100644
--- a/src/metabase/query_processor/util.clj
+++ b/src/metabase/query_processor/util.clj
@@ -9,7 +9,7 @@
[metabase.util.schema :as su]
[schema.core :as s]))
-;; TODO - I think most of the functions in this namespace that we don't remove could be moved to `metabase.mbql.util`
+;; TODO - I think most of the functions in this namespace that we don't remove could be moved to [[metabase.mbql.util]]
(defn ^:deprecated mbql-query? ;; not really needed anymore since we don't need to normalize tokens
"Is the given query an MBQL query?
@@ -22,7 +22,7 @@
[{{aggregations :aggregation, :keys [limit page]} :query}]
(and (not limit)
(not page)
- (nil? aggregations)))
+ (empty? aggregations)))
(defn default-query->remark
"Generates the default query remark. Exists as a separate function so that overrides of the query->remark multimethod
@@ -39,7 +39,7 @@
(defmulti query->remark
"Generate an appropriate remark `^String` to be prepended to a query to give DBAs additional information about the query
- being executed. See documentation for `mbql->native` and #2386.
+ being executed. See documentation for [[metabase.driver/mbql->native]] and #2386.
for more information.
So this turns your average 10, 20, 30 character query into a 110, 120, 130 etc character query.
@@ -48,8 +48,7 @@
'Hey, this is a 20 character query! What's it talking about, error at position 120?'
This gets fixed, but in a spooky-action-at-a-distance way, in
- `frontend/src/metabase/query_builder/components/VisualizationError.jsx`
- "
+ `frontend/src/metabase/query_builder/components/VisualizationError.jsx`"
{:arglists '(^String [driver query])}
driver/dispatch-on-initialized-driver
:hierarchy #'driver/hierarchy)
diff --git a/test/metabase/query_processor/middleware/add_rows_truncated_test.clj b/test/metabase/query_processor/middleware/add_rows_truncated_test.clj
index bf0c291b1a3..1cbe9c41e98 100644
--- a/test/metabase/query_processor/middleware/add_rows_truncated_test.clj
+++ b/test/metabase/query_processor/middleware/add_rows_truncated_test.clj
@@ -6,7 +6,7 @@
(defn- add-rows-truncated [query rows]
(:result
- (mt/test-qp-middleware add-rows-truncated/add-rows-truncated query rows)))
+ (mt/test-qp-middleware add-rows-truncated/add-rows-truncated-middleware query rows)))
(deftest add-rows-truncated-test
(testing "the default behavior is to treat the query as no aggregation and use :max-results-bare-rows"
diff --git a/test/metabase/query_processor/middleware/limit_test.clj b/test/metabase/query_processor/middleware/limit_test.clj
index bbd28cfc956..fc6f2091917 100644
--- a/test/metabase/query_processor/middleware/limit_test.clj
+++ b/test/metabase/query_processor/middleware/limit_test.clj
@@ -38,6 +38,7 @@
":row_count should match the limit added by middleware")
(is (= {:constraints {:max-results 46}
:type :query
- :query {:limit 46}}
+ :query {:limit 46
+ ::limit/original-limit nil}}
(#'limit/add-default-limit query))
"Preprocessed query should have :limit added to it"))))
diff --git a/test/metabase/query_processor/util/nest_query_test.clj b/test/metabase/query_processor/util/nest_query_test.clj
index 05df20391a0..92abf605b1d 100644
--- a/test/metabase/query_processor/util/nest_query_test.clj
+++ b/test/metabase/query_processor/util/nest_query_test.clj
@@ -27,180 +27,180 @@
remove-source-metadata))))
(deftest nest-expressions-test
- (is (query= (mt/$ids venues
- {:source-query {:source-table $$venues
- :expressions {"double_price" [:* [:field %price {::add/source-table $$venues
- ::add/source-alias "PRICE"
- ::add/desired-alias "PRICE"
- ::add/position 5}]
- 2]}
- :fields [[:field %id {::add/source-table $$venues
- ::add/source-alias "ID"
- ::add/desired-alias "ID"
- ::add/position 0}]
- [:field %name {::add/source-table $$venues
- ::add/source-alias "NAME"
- ::add/desired-alias "NAME"
- ::add/position 1}]
- [:field %category_id {::add/source-table $$venues
- ::add/source-alias "CATEGORY_ID"
- ::add/desired-alias "CATEGORY_ID"
- ::add/position 2}]
- [:field %latitude {::add/source-table $$venues
- ::add/source-alias "LATITUDE"
- ::add/desired-alias "LATITUDE"
- ::add/position 3}]
- [:field %longitude {::add/source-table $$venues
- ::add/source-alias "LONGITUDE"
- ::add/desired-alias "LONGITUDE"
- ::add/position 4}]
- [:field %price {::add/source-table $$venues
- ::add/source-alias "PRICE"
- ::add/desired-alias "PRICE"
- ::add/position 5}]
- [:expression "double_price" {::add/desired-alias "double_price"
- ::add/position 6}]]}
- :breakout [[:field %price {::add/source-table ::add/source
- ::add/source-alias "PRICE"
- ::add/desired-alias "PRICE"
- ::add/position 0}]]
- :aggregation [[:aggregation-options [:count] {:name "count"
- ::add/desired-alias "count"
- ::add/position 1}]]
- :fields [[:field "double_price" {:base-type :type/Float
- ::add/source-table ::add/source
- ::add/source-alias "double_price"
- ::add/desired-alias "double_price"
- ::add/position 2}]]
- :order-by [[:asc [:field %price {::add/source-table ::add/source
- ::add/source-alias "PRICE"
- ::add/desired-alias "PRICE"
- ::add/position 0}]]]})
- (nest-expressions
- (mt/mbql-query venues
- {:expressions {"double_price" [:* $price 2]}
- :breakout [$price]
- :aggregation [[:count]]
- :fields [[:expression "double_price"]]})))))
+ (is (partial= (mt/$ids venues
+ {:source-query {:source-table $$venues
+ :expressions {"double_price" [:* [:field %price {::add/source-table $$venues
+ ::add/source-alias "PRICE"
+ ::add/desired-alias "PRICE"
+ ::add/position 5}]
+ 2]}
+ :fields [[:field %id {::add/source-table $$venues
+ ::add/source-alias "ID"
+ ::add/desired-alias "ID"
+ ::add/position 0}]
+ [:field %name {::add/source-table $$venues
+ ::add/source-alias "NAME"
+ ::add/desired-alias "NAME"
+ ::add/position 1}]
+ [:field %category_id {::add/source-table $$venues
+ ::add/source-alias "CATEGORY_ID"
+ ::add/desired-alias "CATEGORY_ID"
+ ::add/position 2}]
+ [:field %latitude {::add/source-table $$venues
+ ::add/source-alias "LATITUDE"
+ ::add/desired-alias "LATITUDE"
+ ::add/position 3}]
+ [:field %longitude {::add/source-table $$venues
+ ::add/source-alias "LONGITUDE"
+ ::add/desired-alias "LONGITUDE"
+ ::add/position 4}]
+ [:field %price {::add/source-table $$venues
+ ::add/source-alias "PRICE"
+ ::add/desired-alias "PRICE"
+ ::add/position 5}]
+ [:expression "double_price" {::add/desired-alias "double_price"
+ ::add/position 6}]]}
+ :breakout [[:field %price {::add/source-table ::add/source
+ ::add/source-alias "PRICE"
+ ::add/desired-alias "PRICE"
+ ::add/position 0}]]
+ :aggregation [[:aggregation-options [:count] {:name "count"
+ ::add/desired-alias "count"
+ ::add/position 1}]]
+ :fields [[:field "double_price" {:base-type :type/Float
+ ::add/source-table ::add/source
+ ::add/source-alias "double_price"
+ ::add/desired-alias "double_price"
+ ::add/position 2}]]
+ :order-by [[:asc [:field %price {::add/source-table ::add/source
+ ::add/source-alias "PRICE"
+ ::add/desired-alias "PRICE"
+ ::add/position 0}]]]})
+ (nest-expressions
+ (mt/mbql-query venues
+ {:expressions {"double_price" [:* $price 2]}
+ :breakout [$price]
+ :aggregation [[:count]]
+ :fields [[:expression "double_price"]]})))))
(deftest nest-expressions-with-existing-non-expression-fields-test
(testing "Other `:fields` besides the `:expressions` should be preserved in the top level"
- (is (query= (mt/$ids checkins
- {:source-query {:source-table $$checkins
- :expressions {"double_id" [:*
- [:field %checkins.id {::add/source-table $$checkins
- ::add/source-alias "ID"
- ::add/desired-alias "ID"
- ::add/position 0}]
- 2]}
- :fields [[:field %id {::add/source-table $$checkins
- ::add/source-alias "ID"
- ::add/desired-alias "ID"
- ::add/position 0}]
- [:field %date {:temporal-unit :default
- ::add/source-table $$checkins
- ::add/source-alias "DATE"
- ::add/desired-alias "DATE"
- ::add/position 1}]
- [:field %user_id {::add/source-table $$checkins
- ::add/source-alias "USER_ID"
- ::add/desired-alias "USER_ID"
- ::add/position 2}]
- [:field %venue_id {::add/source-table $$checkins
- ::add/source-alias "VENUE_ID"
- ::add/desired-alias "VENUE_ID"
- ::add/position 3}]
- [:expression "double_id" {::add/desired-alias "double_id"
- ::add/position 4}]]}
- :fields [[:field "double_id" {:base-type :type/Float
- ::add/source-table ::add/source
- ::add/source-alias "double_id"
- ::add/desired-alias "double_id"
- ::add/position 0}]
- [:field %date {:temporal-unit :day
- ::nest-query/outer-select true
- ::add/source-table ::add/source
- ::add/source-alias "DATE"
- ::add/desired-alias "DATE"
- ::add/position 1}]
- [:field %date {:temporal-unit :month
- ::nest-query/outer-select true
- ::add/source-table ::add/source
- ::add/source-alias "DATE"
- ::add/desired-alias "DATE_2"
- ::add/position 2}]]
- :limit 1})
- (nest-expressions
- (mt/mbql-query checkins
- {:expressions {"double_id" [:* $id 2]}
- :fields [[:expression "double_id"]
- !day.date
- !month.date]
- :limit 1}))))))
+ (is (partial= (mt/$ids checkins
+ {:source-query {:source-table $$checkins
+ :expressions {"double_id" [:*
+ [:field %checkins.id {::add/source-table $$checkins
+ ::add/source-alias "ID"
+ ::add/desired-alias "ID"
+ ::add/position 0}]
+ 2]}
+ :fields [[:field %id {::add/source-table $$checkins
+ ::add/source-alias "ID"
+ ::add/desired-alias "ID"
+ ::add/position 0}]
+ [:field %date {:temporal-unit :default
+ ::add/source-table $$checkins
+ ::add/source-alias "DATE"
+ ::add/desired-alias "DATE"
+ ::add/position 1}]
+ [:field %user_id {::add/source-table $$checkins
+ ::add/source-alias "USER_ID"
+ ::add/desired-alias "USER_ID"
+ ::add/position 2}]
+ [:field %venue_id {::add/source-table $$checkins
+ ::add/source-alias "VENUE_ID"
+ ::add/desired-alias "VENUE_ID"
+ ::add/position 3}]
+ [:expression "double_id" {::add/desired-alias "double_id"
+ ::add/position 4}]]}
+ :fields [[:field "double_id" {:base-type :type/Float
+ ::add/source-table ::add/source
+ ::add/source-alias "double_id"
+ ::add/desired-alias "double_id"
+ ::add/position 0}]
+ [:field %date {:temporal-unit :day
+ ::nest-query/outer-select true
+ ::add/source-table ::add/source
+ ::add/source-alias "DATE"
+ ::add/desired-alias "DATE"
+ ::add/position 1}]
+ [:field %date {:temporal-unit :month
+ ::nest-query/outer-select true
+ ::add/source-table ::add/source
+ ::add/source-alias "DATE"
+ ::add/desired-alias "DATE_2"
+ ::add/position 2}]]
+ :limit 1})
+ (nest-expressions
+ (mt/mbql-query checkins
+ {:expressions {"double_id" [:* $id 2]}
+ :fields [[:expression "double_id"]
+ !day.date
+ !month.date]
+ :limit 1}))))))
(deftest multiple-expressions-test
(testing "Make sure the nested version of the query doesn't mix up expressions if we have ones that reference others"
- (is (query= (mt/$ids venues
- {:source-query {:source-table $$venues
- :expressions {"big_price"
- [:+
- [:field %price {::add/position 5
- ::add/source-table $$venues
- ::add/source-alias "PRICE"
- ::add/desired-alias "PRICE"}]
- 2]
+ (is (partial= (mt/$ids venues
+ {:source-query {:source-table $$venues
+ :expressions {"big_price"
+ [:+
+ [:field %price {::add/position 5
+ ::add/source-table $$venues
+ ::add/source-alias "PRICE"
+ ::add/desired-alias "PRICE"}]
+ 2]
- "my_cool_new_field"
- [:/
- [:field %price {::add/position 5
- ::add/source-table $$venues
- ::add/source-alias "PRICE"
- ::add/desired-alias "PRICE"}]
- [:expression "big_price" {::add/position 6
- ::add/desired-alias "big_price"}]]}
- :fields [[:field %id {::add/position 0
- ::add/source-table $$venues
- ::add/source-alias "ID"
- ::add/desired-alias "ID"}]
- [:field %name {::add/position 1
+ "my_cool_new_field"
+ [:/
+ [:field %price {::add/position 5
+ ::add/source-table $$venues
+ ::add/source-alias "PRICE"
+ ::add/desired-alias "PRICE"}]
+ [:expression "big_price" {::add/position 6
+ ::add/desired-alias "big_price"}]]}
+ :fields [[:field %id {::add/position 0
::add/source-table $$venues
- ::add/source-alias "NAME"
- ::add/desired-alias "NAME"}]
- [:field %category_id {::add/position 2
+ ::add/source-alias "ID"
+ ::add/desired-alias "ID"}]
+ [:field %name {::add/position 1
+ ::add/source-table $$venues
+ ::add/source-alias "NAME"
+ ::add/desired-alias "NAME"}]
+ [:field %category_id {::add/position 2
+ ::add/source-table $$venues
+ ::add/source-alias "CATEGORY_ID"
+ ::add/desired-alias "CATEGORY_ID"}]
+ [:field %latitude {::add/position 3
+ ::add/source-table $$venues
+ ::add/source-alias "LATITUDE"
+ ::add/desired-alias "LATITUDE"}]
+ [:field %longitude {::add/position 4
::add/source-table $$venues
- ::add/source-alias "CATEGORY_ID"
- ::add/desired-alias "CATEGORY_ID"}]
- [:field %latitude {::add/position 3
- ::add/source-table $$venues
- ::add/source-alias "LATITUDE"
- ::add/desired-alias "LATITUDE"}]
- [:field %longitude {::add/position 4
- ::add/source-table $$venues
- ::add/source-alias "LONGITUDE"
- ::add/desired-alias "LONGITUDE"}]
- [:field %price {::add/position 5
- ::add/source-table $$venues
- ::add/source-alias "PRICE"
- ::add/desired-alias "PRICE"}]
- [:expression "big_price" {::add/position 6
- ::add/desired-alias "big_price"}]
- [:expression "my_cool_new_field" {::add/position 7
- ::add/desired-alias "my_cool_new_field"}]]}
- :fields [[:field "my_cool_new_field" {:base-type :type/Float
- ::add/position 0
- ::add/source-table ::add/source
- ::add/source-alias "my_cool_new_field"
- ::add/desired-alias "my_cool_new_field"}]]
- :order-by [[:asc [:field %id {::add/source-table ::add/source
- ::add/source-alias "ID"}]]]
- :limit 3})
- (nest-expressions
- (mt/mbql-query venues
- {:expressions {"big_price" [:+ $price 2]
- "my_cool_new_field" [:/ $price [:expression "big_price"]]}
- :fields [[:expression "my_cool_new_field"]]
- :order-by [[:asc $id]]
- :limit 3}))))))
+ ::add/source-alias "LONGITUDE"
+ ::add/desired-alias "LONGITUDE"}]
+ [:field %price {::add/position 5
+ ::add/source-table $$venues
+ ::add/source-alias "PRICE"
+ ::add/desired-alias "PRICE"}]
+ [:expression "big_price" {::add/position 6
+ ::add/desired-alias "big_price"}]
+ [:expression "my_cool_new_field" {::add/position 7
+ ::add/desired-alias "my_cool_new_field"}]]}
+ :fields [[:field "my_cool_new_field" {:base-type :type/Float
+ ::add/position 0
+ ::add/source-table ::add/source
+ ::add/source-alias "my_cool_new_field"
+ ::add/desired-alias "my_cool_new_field"}]]
+ :order-by [[:asc [:field %id {::add/source-table ::add/source
+ ::add/source-alias "ID"}]]]
+ :limit 3})
+ (nest-expressions
+ (mt/mbql-query venues
+ {:expressions {"big_price" [:+ $price 2]
+ "my_cool_new_field" [:/ $price [:expression "big_price"]]}
+ :fields [[:expression "my_cool_new_field"]]
+ :order-by [[:asc $id]]
+ :limit 3}))))))
(deftest nest-expressions-ignore-source-queries-test
(testing (str "When 'raising' :expression clauses, only raise ones in the current level. Handle duplicate expression "
@@ -213,259 +213,125 @@
:fields [$id [:expression "x"]]
:limit 1})]
(mt/with-native-query-testing-context query
- (is (query= (mt/$ids venues
- {:fields [[:field %id {::add/source-table ::add/source
- ::add/source-alias "ID"
- ::add/desired-alias "ID"
- ::add/position 0}]
- [:field "x_2" {:base-type :type/Float
- ::add/source-table ::add/source
- ::add/source-alias "x_2"
- ::add/desired-alias "x_2"
- ::add/position 1}]]
- :source-query {:expressions {"x" [:*
- [:field %price {::add/source-table ::add/source
- ::add/source-alias "PRICE"}]
- 4]}
- :fields [[:field %id {::add/source-table ::add/source
- ::add/source-alias "ID"
- ::add/desired-alias "ID"
- ::add/position 0}]
- [:field "x" {:base-type :type/Float
- ::add/source-table ::add/source
- ::add/source-alias "x"
- ::add/desired-alias "x"
- ::add/position 1}]
- [:expression "x" {::add/desired-alias "x_2"
- ::add/position 2}]]
- :source-query {:source-table $$venues
- :expressions {"x" [:*
- [:field %price {::add/source-table $$venues
- ::add/source-alias "PRICE"}]
- 2]}
- :fields [[:field %id {::add/source-table $$venues
- ::add/source-alias "ID"
- ::add/desired-alias "ID"
- ::add/position 0}]
- [:expression "x" {::add/desired-alias "x"
- ::add/position 1}]]}}
- :limit 1})
- (nest-expressions query)))))))
+ (is (partial= (mt/$ids venues
+ {:fields [[:field %id {::add/source-table ::add/source
+ ::add/source-alias "ID"
+ ::add/desired-alias "ID"
+ ::add/position 0}]
+ [:field "x_2" {:base-type :type/Float
+ ::add/source-table ::add/source
+ ::add/source-alias "x_2"
+ ::add/desired-alias "x_2"
+ ::add/position 1}]]
+ :source-query {:expressions {"x" [:*
+ [:field %price {::add/source-table ::add/source
+ ::add/source-alias "PRICE"}]
+ 4]}
+ :fields [[:field %id {::add/source-table ::add/source
+ ::add/source-alias "ID"
+ ::add/desired-alias "ID"
+ ::add/position 0}]
+ [:field "x" {:base-type :type/Float
+ ::add/source-table ::add/source
+ ::add/source-alias "x"
+ ::add/desired-alias "x"
+ ::add/position 1}]
+ [:expression "x" {::add/desired-alias "x_2"
+ ::add/position 2}]]
+ :source-query {:source-table $$venues
+ :expressions {"x" [:*
+ [:field %price {::add/source-table $$venues
+ ::add/source-alias "PRICE"}]
+ 2]}
+ :fields [[:field %id {::add/source-table $$venues
+ ::add/source-alias "ID"
+ ::add/desired-alias "ID"
+ ::add/position 0}]
+ [:expression "x" {::add/desired-alias "x"
+ ::add/position 1}]]}}
+ :limit 1})
+ (nest-expressions query)))))))
(deftest nest-expressions-with-joins-test
(testing "If there are any `:joins`, those need to be nested into the `:source-query` as well."
- (is (query= (mt/$ids venues
- {:source-query {:source-table $$venues
- :joins [{:strategy :left-join
- :condition [:=
- [:field %category_id {::add/source-table $$venues
- ::add/source-alias "CATEGORY_ID"
- ::add/desired-alias "CATEGORY_ID"
- ::add/position 2}]
- [:field %category_id {:join-alias "CategoriesStats"
+ (is (partial= (mt/$ids venues
+ {:source-query {:source-table $$venues
+ :joins [{:strategy :left-join
+ :condition [:=
+ [:field %category_id {::add/source-table $$venues
+ ::add/source-alias "CATEGORY_ID"
+ ::add/desired-alias "CATEGORY_ID"
+ ::add/position 2}]
+ [:field %category_id {:join-alias "CategoriesStats"
+ ::add/source-table "CategoriesStats"
+ ::add/source-alias "CATEGORY_ID"
+ ::add/desired-alias "CategoriesStats__CATEGORY_ID"
+ ::add/position 7}]]
+ :source-query {:source-table $$venues
+ :aggregation [[:aggregation-options
+ [:max [:field %price {::add/source-table $$venues
+ ::add/source-alias "PRICE"}]]
+ {:name "MaxPrice"
+ ::add/desired-alias "MaxPrice"
+ ::add/position 1}]
+ [:aggregation-options
+ [:avg
+ [:field
+ %price
+ {::add/source-table $$venues
+ ::add/source-alias "PRICE"}]]
+ {:name "AvgPrice"
+ ::add/desired-alias "AvgPrice"
+ ::add/position 2}]
+ [:aggregation-options
+ [:min [:field %price {::add/source-table $$venues
+ ::add/source-alias "PRICE"}]]
+ {:name "MinPrice"
+ ::add/desired-alias "MinPrice"
+ ::add/position 3}]]
+ :breakout [[:field %category_id {::add/source-table $$venues
+ ::add/source-alias "CATEGORY_ID"
+ ::add/desired-alias "CATEGORY_ID"
+ ::add/position 0}]]
+ :order-by [[:asc [:field %category_id {::add/source-table $$venues
+ ::add/source-alias "CATEGORY_ID"
+ ::add/desired-alias "CATEGORY_ID"
+ ::add/position 0}]]]}
+ :alias "CategoriesStats"
+ :fields [[:field %category_id {:join-alias "CategoriesStats"
+ ::add/source-table "CategoriesStats"
+ ::add/source-alias "CATEGORY_ID"
+ ::add/desired-alias "CategoriesStats__CATEGORY_ID"
+ ::add/position 7}]
+ [:field "MaxPrice" {:base-type :type/Integer
+ :join-alias "CategoriesStats"
::add/source-table "CategoriesStats"
- ::add/source-alias "CATEGORY_ID"
- ::add/desired-alias "CategoriesStats__CATEGORY_ID"
- ::add/position 7}]]
- :source-query {:source-table $$venues
- :aggregation [[:aggregation-options
- [:max [:field %price {::add/source-table $$venues
- ::add/source-alias "PRICE"}]]
- {:name "MaxPrice"
- ::add/desired-alias "MaxPrice"
- ::add/position 1}]
- [:aggregation-options
- [:avg
- [:field
- %price
- {::add/source-table $$venues
- ::add/source-alias "PRICE"}]]
- {:name "AvgPrice"
- ::add/desired-alias "AvgPrice"
- ::add/position 2}]
- [:aggregation-options
- [:min [:field %price {::add/source-table $$venues
- ::add/source-alias "PRICE"}]]
- {:name "MinPrice"
- ::add/desired-alias "MinPrice"
- ::add/position 3}]]
- :breakout [[:field %category_id {::add/source-table $$venues
- ::add/source-alias "CATEGORY_ID"
- ::add/desired-alias "CATEGORY_ID"
- ::add/position 0}]]
- :order-by [[:asc [:field %category_id {::add/source-table $$venues
- ::add/source-alias "CATEGORY_ID"
- ::add/desired-alias "CATEGORY_ID"
- ::add/position 0}]]]}
- :alias "CategoriesStats"
- :fields [[:field %category_id {:join-alias "CategoriesStats"
+ ::add/source-alias "MaxPrice"
+ ::add/desired-alias "CategoriesStats__MaxPrice"
+ ::add/position 8}]
+ [:field "AvgPrice" {:base-type :type/Integer
+ :join-alias "CategoriesStats"
::add/source-table "CategoriesStats"
- ::add/source-alias "CATEGORY_ID"
- ::add/desired-alias "CategoriesStats__CATEGORY_ID"
- ::add/position 7}]
- [:field "MaxPrice" {:base-type :type/Integer
- :join-alias "CategoriesStats"
- ::add/source-table "CategoriesStats"
- ::add/source-alias "MaxPrice"
- ::add/desired-alias "CategoriesStats__MaxPrice"
- ::add/position 8}]
- [:field "AvgPrice" {:base-type :type/Integer
- :join-alias "CategoriesStats"
- ::add/source-table "CategoriesStats"
- ::add/source-alias "AvgPrice"
- ::add/desired-alias "CategoriesStats__AvgPrice"
- ::add/position 9}]
- [:field "MinPrice" {:base-type :type/Integer
- :join-alias "CategoriesStats"
- ::add/source-table "CategoriesStats"
- ::add/source-alias "MinPrice"
- ::add/desired-alias "CategoriesStats__MinPrice"
- ::add/position 10}]]}]
- :expressions {"RelativePrice" [:/
- [:field %price {::add/source-table $$venues
- ::add/source-alias "PRICE"
- ::add/desired-alias "PRICE"
- ::add/position 5}]
- [:field "AvgPrice" {:base-type :type/Integer
- :join-alias "CategoriesStats"
- ::add/source-table "CategoriesStats"
- ::add/source-alias "AvgPrice"
- ::add/desired-alias "CategoriesStats__AvgPrice"
- ::add/position 9}]]}
- :fields [[:field %id {::add/source-table $$venues
- ::add/source-alias "ID"
- ::add/desired-alias "ID"
- ::add/position 0}]
- [:field %name {::add/source-table $$venues
- ::add/source-alias "NAME"
- ::add/desired-alias "NAME"
- ::add/position 1}]
- [:field %category_id {::add/source-table $$venues
- ::add/source-alias "CATEGORY_ID"
- ::add/desired-alias "CATEGORY_ID"
- ::add/position 2}]
- [:field %latitude {::add/source-table $$venues
- ::add/source-alias "LATITUDE"
- ::add/desired-alias "LATITUDE"
- ::add/position 3}]
- [:field %longitude {::add/source-table $$venues
- ::add/source-alias "LONGITUDE"
- ::add/desired-alias "LONGITUDE"
- ::add/position 4}]
- [:field %price {::add/source-table $$venues
- ::add/source-alias "PRICE"
- ::add/desired-alias "PRICE"
- ::add/position 5}]
- [:expression "RelativePrice" {::add/desired-alias "RelativePrice"
- ::add/position 6}]
- [:field %category_id {:join-alias "CategoriesStats"
- ::add/source-table "CategoriesStats"
- ::add/source-alias "CATEGORY_ID"
- ::add/desired-alias "CategoriesStats__CATEGORY_ID"
- ::add/position 7}]
- [:field "MaxPrice" {:base-type :type/Integer
- :join-alias "CategoriesStats"
- ::add/source-table "CategoriesStats"
- ::add/source-alias "MaxPrice"
- ::add/desired-alias "CategoriesStats__MaxPrice"
- ::add/position 8}]
- [:field "AvgPrice" {:base-type :type/Integer
- :join-alias "CategoriesStats"
- ::add/source-table "CategoriesStats"
- ::add/source-alias "AvgPrice"
- ::add/desired-alias "CategoriesStats__AvgPrice"
- ::add/position 9}]
- [:field "MinPrice" {:base-type :type/Integer
- :join-alias "CategoriesStats"
- ::add/source-table "CategoriesStats"
- ::add/source-alias "MinPrice"
- ::add/desired-alias "CategoriesStats__MinPrice"
- ::add/position 10}]]}
- :fields [[:field %id {::add/source-table ::add/source
- ::add/source-alias "ID"
- ::add/desired-alias "ID"
- ::add/position 0}]
- [:field %name {::add/source-table ::add/source
- ::add/source-alias "NAME"
- ::add/desired-alias "NAME"
- ::add/position 1}]
- [:field %category_id {::add/source-table ::add/source
- ::add/source-alias "CATEGORY_ID"
- ::add/desired-alias "CATEGORY_ID"
- ::add/position 2}]
- [:field %latitude {::add/source-table ::add/source
- ::add/source-alias "LATITUDE"
- ::add/desired-alias "LATITUDE"
- ::add/position 3}]
- [:field %longitude {::add/source-table ::add/source
- ::add/source-alias "LONGITUDE"
- ::add/desired-alias "LONGITUDE"
- ::add/position 4}]
- [:field %price {::add/source-table ::add/source
- ::add/source-alias "PRICE"
- ::add/desired-alias "PRICE"
- ::add/position 5}]
- [:field "RelativePrice" {:base-type :type/Float
- ::add/source-table ::add/source
- ::add/source-alias "RelativePrice"
- ::add/desired-alias "RelativePrice"
- ::add/position 6}]
- [:field %category_id {:join-alias "CategoriesStats"
- ::add/source-alias "CategoriesStats__CATEGORY_ID"
- ::add/desired-alias "CategoriesStats__CATEGORY_ID"
- ::add/source-table ::add/source
- ::add/position 7}]
- [:field "MaxPrice" {:base-type :type/Integer
- :join-alias "CategoriesStats"
- ::add/source-alias "CategoriesStats__MaxPrice"
- ::add/desired-alias "CategoriesStats__MaxPrice"
- ::add/source-table ::add/source
- ::add/position 8}]
- [:field "AvgPrice" {:base-type :type/Integer
- :join-alias "CategoriesStats"
- ::add/source-alias "CategoriesStats__AvgPrice"
- ::add/desired-alias "CategoriesStats__AvgPrice"
- ::add/source-table ::add/source
- ::add/position 9}]
- [:field "MinPrice" {:base-type :type/Integer
- :join-alias "CategoriesStats"
- ::add/source-alias "CategoriesStats__MinPrice"
- ::add/desired-alias "CategoriesStats__MinPrice"
- ::add/source-table ::add/source
- ::add/position 10}]]
- :limit 3})
- (nest-expressions
- (mt/mbql-query venues
- {:fields [$id
- $name
- $category_id
- $latitude
- $longitude
- $price
- [:expression "RelativePrice"]
- &CategoriesStats.category_id
- &CategoriesStats.*MaxPrice/Integer
- &CategoriesStats.*AvgPrice/Integer
- &CategoriesStats.*MinPrice/Integer]
- :expressions {"RelativePrice" [:/ $price &CategoriesStats.*AvgPrice/Integer]}
- :joins [{:strategy :left-join
- :condition [:= $category_id &CategoriesStats.category_id]
- :source-query {:source-table $$venues
- :aggregation [[:aggregation-options [:max $price] {:name "MaxPrice"}]
- [:aggregation-options [:avg $price] {:name "AvgPrice"}]
- [:aggregation-options [:min $price] {:name "MinPrice"}]]
- :breakout [$category_id]}
- :alias "CategoriesStats"
- :fields :all}]
- :limit 3}))))))
-
-(deftest nest-expressions-eliminate-duplicate-coercion-test
- (testing "If coercion happens in the source query, don't do it a second time in the parent query (#12430)"
- (mt/with-temp-vals-in-db Field (mt/id :venues :price) {:coercion_strategy :Coercion/UNIXSeconds->DateTime
- :effective_type :type/DateTime}
- (is (query= (mt/$ids venues
- {:source-query {:source-table $$venues
- :expressions {"test" [:* 1 1]}
+ ::add/source-alias "AvgPrice"
+ ::add/desired-alias "CategoriesStats__AvgPrice"
+ ::add/position 9}]
+ [:field "MinPrice" {:base-type :type/Integer
+ :join-alias "CategoriesStats"
+ ::add/source-table "CategoriesStats"
+ ::add/source-alias "MinPrice"
+ ::add/desired-alias "CategoriesStats__MinPrice"
+ ::add/position 10}]]}]
+ :expressions {"RelativePrice" [:/
+ [:field %price {::add/source-table $$venues
+ ::add/source-alias "PRICE"
+ ::add/desired-alias "PRICE"
+ ::add/position 5}]
+ [:field "AvgPrice" {:base-type :type/Integer
+ :join-alias "CategoriesStats"
+ ::add/source-table "CategoriesStats"
+ ::add/source-alias "AvgPrice"
+ ::add/desired-alias "CategoriesStats__AvgPrice"
+ ::add/position 9}]]}
:fields [[:field %id {::add/source-table $$venues
::add/source-alias "ID"
::add/desired-alias "ID"
@@ -486,211 +352,345 @@
::add/source-alias "LONGITUDE"
::add/desired-alias "LONGITUDE"
::add/position 4}]
- [:field %price {:temporal-unit :default
- ::add/source-table $$venues
+ [:field %price {::add/source-table $$venues
::add/source-alias "PRICE"
::add/desired-alias "PRICE"
::add/position 5}]
- [:expression "test" {::add/desired-alias "test"
- ::add/position 6}]]}
- :fields [[:field %price {:temporal-unit :default
- ::nest-query/outer-select true
- ::add/source-table ::add/source
- ::add/source-alias "PRICE"
- ::add/desired-alias "PRICE"
- ::add/position 0}]
- [:field "test" {:base-type :type/Float
- ::add/source-table ::add/source
- ::add/source-alias "test"
- ::add/desired-alias "test"
- ::add/position 1}]]
- :limit 1})
+ [:expression "RelativePrice" {::add/desired-alias "RelativePrice"
+ ::add/position 6}]
+ [:field %category_id {:join-alias "CategoriesStats"
+ ::add/source-table "CategoriesStats"
+ ::add/source-alias "CATEGORY_ID"
+ ::add/desired-alias "CategoriesStats__CATEGORY_ID"
+ ::add/position 7}]
+ [:field "MaxPrice" {:base-type :type/Integer
+ :join-alias "CategoriesStats"
+ ::add/source-table "CategoriesStats"
+ ::add/source-alias "MaxPrice"
+ ::add/desired-alias "CategoriesStats__MaxPrice"
+ ::add/position 8}]
+ [:field "AvgPrice" {:base-type :type/Integer
+ :join-alias "CategoriesStats"
+ ::add/source-table "CategoriesStats"
+ ::add/source-alias "AvgPrice"
+ ::add/desired-alias "CategoriesStats__AvgPrice"
+ ::add/position 9}]
+ [:field "MinPrice" {:base-type :type/Integer
+ :join-alias "CategoriesStats"
+ ::add/source-table "CategoriesStats"
+ ::add/source-alias "MinPrice"
+ ::add/desired-alias "CategoriesStats__MinPrice"
+ ::add/position 10}]]}
+ :fields [[:field %id {::add/source-table ::add/source
+ ::add/source-alias "ID"
+ ::add/desired-alias "ID"
+ ::add/position 0}]
+ [:field %name {::add/source-table ::add/source
+ ::add/source-alias "NAME"
+ ::add/desired-alias "NAME"
+ ::add/position 1}]
+ [:field %category_id {::add/source-table ::add/source
+ ::add/source-alias "CATEGORY_ID"
+ ::add/desired-alias "CATEGORY_ID"
+ ::add/position 2}]
+ [:field %latitude {::add/source-table ::add/source
+ ::add/source-alias "LATITUDE"
+ ::add/desired-alias "LATITUDE"
+ ::add/position 3}]
+ [:field %longitude {::add/source-table ::add/source
+ ::add/source-alias "LONGITUDE"
+ ::add/desired-alias "LONGITUDE"
+ ::add/position 4}]
+ [:field %price {::add/source-table ::add/source
+ ::add/source-alias "PRICE"
+ ::add/desired-alias "PRICE"
+ ::add/position 5}]
+ [:field "RelativePrice" {:base-type :type/Float
+ ::add/source-table ::add/source
+ ::add/source-alias "RelativePrice"
+ ::add/desired-alias "RelativePrice"
+ ::add/position 6}]
+ [:field %category_id {:join-alias "CategoriesStats"
+ ::add/source-alias "CategoriesStats__CATEGORY_ID"
+ ::add/desired-alias "CategoriesStats__CATEGORY_ID"
+ ::add/source-table ::add/source
+ ::add/position 7}]
+ [:field "MaxPrice" {:base-type :type/Integer
+ :join-alias "CategoriesStats"
+ ::add/source-alias "CategoriesStats__MaxPrice"
+ ::add/desired-alias "CategoriesStats__MaxPrice"
+ ::add/source-table ::add/source
+ ::add/position 8}]
+ [:field "AvgPrice" {:base-type :type/Integer
+ :join-alias "CategoriesStats"
+ ::add/source-alias "CategoriesStats__AvgPrice"
+ ::add/desired-alias "CategoriesStats__AvgPrice"
+ ::add/source-table ::add/source
+ ::add/position 9}]
+ [:field "MinPrice" {:base-type :type/Integer
+ :join-alias "CategoriesStats"
+ ::add/source-alias "CategoriesStats__MinPrice"
+ ::add/desired-alias "CategoriesStats__MinPrice"
+ ::add/source-table ::add/source
+ ::add/position 10}]]
+ :limit 3})
(nest-expressions
(mt/mbql-query venues
- {:expressions {"test" ["*" 1 1]}
- :fields [$price
- [:expression "test"]]
- :limit 1})))))))
+ {:fields [$id
+ $name
+ $category_id
+ $latitude
+ $longitude
+ $price
+ [:expression "RelativePrice"]
+ &CategoriesStats.category_id
+ &CategoriesStats.*MaxPrice/Integer
+ &CategoriesStats.*AvgPrice/Integer
+ &CategoriesStats.*MinPrice/Integer]
+ :expressions {"RelativePrice" [:/ $price &CategoriesStats.*AvgPrice/Integer]}
+ :joins [{:strategy :left-join
+ :condition [:= $category_id &CategoriesStats.category_id]
+ :source-query {:source-table $$venues
+ :aggregation [[:aggregation-options [:max $price] {:name "MaxPrice"}]
+ [:aggregation-options [:avg $price] {:name "AvgPrice"}]
+ [:aggregation-options [:min $price] {:name "MinPrice"}]]
+ :breakout [$category_id]}
+ :alias "CategoriesStats"
+ :fields :all}]
+ :limit 3}))))))
+
+(deftest nest-expressions-eliminate-duplicate-coercion-test
+ (testing "If coercion happens in the source query, don't do it a second time in the parent query (#12430)"
+ (mt/with-temp-vals-in-db Field (mt/id :venues :price) {:coercion_strategy :Coercion/UNIXSeconds->DateTime
+ :effective_type :type/DateTime}
+ (is (partial= (mt/$ids venues
+ {:source-query {:source-table $$venues
+ :expressions {"test" [:* 1 1]}
+ :fields [[:field %id {::add/source-table $$venues
+ ::add/source-alias "ID"
+ ::add/desired-alias "ID"
+ ::add/position 0}]
+ [:field %name {::add/source-table $$venues
+ ::add/source-alias "NAME"
+ ::add/desired-alias "NAME"
+ ::add/position 1}]
+ [:field %category_id {::add/source-table $$venues
+ ::add/source-alias "CATEGORY_ID"
+ ::add/desired-alias "CATEGORY_ID"
+ ::add/position 2}]
+ [:field %latitude {::add/source-table $$venues
+ ::add/source-alias "LATITUDE"
+ ::add/desired-alias "LATITUDE"
+ ::add/position 3}]
+ [:field %longitude {::add/source-table $$venues
+ ::add/source-alias "LONGITUDE"
+ ::add/desired-alias "LONGITUDE"
+ ::add/position 4}]
+ [:field %price {:temporal-unit :default
+ ::add/source-table $$venues
+ ::add/source-alias "PRICE"
+ ::add/desired-alias "PRICE"
+ ::add/position 5}]
+ [:expression "test" {::add/desired-alias "test"
+ ::add/position 6}]]}
+ :fields [[:field %price {:temporal-unit :default
+ ::nest-query/outer-select true
+ ::add/source-table ::add/source
+ ::add/source-alias "PRICE"
+ ::add/desired-alias "PRICE"
+ ::add/position 0}]
+ [:field "test" {:base-type :type/Float
+ ::add/source-table ::add/source
+ ::add/source-alias "test"
+ ::add/desired-alias "test"
+ ::add/position 1}]]
+ :limit 1})
+ (nest-expressions
+ (mt/mbql-query venues
+ {:expressions {"test" ["*" 1 1]}
+ :fields [$price
+ [:expression "test"]]
+ :limit 1})))))))
(deftest multiple-joins-with-expressions-test
(testing "We should be able to compile a complicated query with multiple joins and expressions correctly"
(mt/dataset sample-dataset
- (is (query= (mt/$ids orders
- (merge {:source-query (let [id [:field %id {::add/source-table $$orders
- ::add/source-alias "ID"
- ::add/desired-alias "ID"
- ::add/position 0}]
- user-id [:field %user_id {::add/source-table $$orders
- ::add/source-alias "USER_ID"
- ::add/desired-alias "USER_ID"
- ::add/position 1}]
- product-id [:field %product_id {::add/source-table $$orders
- ::add/source-alias "PRODUCT_ID"
- ::add/desired-alias "PRODUCT_ID"
- ::add/position 2}]
- subtotal [:field %subtotal {::add/source-table $$orders
- ::add/source-alias "SUBTOTAL"
- ::add/desired-alias "SUBTOTAL"
- ::add/position 3}]
- tax [:field %tax {::add/source-table $$orders
- ::add/source-alias "TAX"
- ::add/desired-alias "TAX"
- ::add/position 4}]
- total [:field %total {::add/source-table $$orders
- ::add/source-alias "TOTAL"
- ::add/desired-alias "TOTAL"
- ::add/position 5}]
- discount [:field %discount {::add/source-table $$orders
- ::add/source-alias "DISCOUNT"
- ::add/desired-alias "DISCOUNT"
- ::add/position 6}]
- created-at [:field %created_at {:temporal-unit :default
- ::add/source-table $$orders
- ::add/source-alias "CREATED_AT"
- ::add/desired-alias "CREATED_AT"
- ::add/position 7}]
- quantity [:field %quantity {::add/source-table $$orders
- ::add/source-alias "QUANTITY"
- ::add/desired-alias "QUANTITY"
- ::add/position 8}]
- pivot-grouping [:expression "pivot-grouping" {::add/desired-alias "pivot-grouping"
- ::add/position 9}]
- products-category [:field %products.category {:join-alias "PRODUCTS__via__PRODUCT_ID"
- ::add/source-table "PRODUCTS__via__PRODUCT_ID"
- ::add/source-alias "CATEGORY"
- ::add/desired-alias "PRODUCTS__via__PRODUCT_ID__CATEGORY"
- ::add/position 10}]
- products-id [:field %products.id {:join-alias "PRODUCTS__via__PRODUCT_ID"
- ::add/source-table "PRODUCTS__via__PRODUCT_ID"
- ::add/source-alias "ID"
- ::add/desired-alias "PRODUCTS__via__PRODUCT_ID__ID"
- ::add/position 11}]]
- {:source-table $$orders
- :joins [{:source-table $$products
- :alias "PRODUCTS__via__PRODUCT_ID"
- :condition [:= product-id products-id]
- :strategy :left-join
- :fk-field-id %product_id}]
- :expressions {"pivot-grouping" [:abs 0]}
- :fields [id
- user-id
- product-id
- subtotal
- tax
- total
- discount
- created-at
- quantity
- pivot-grouping
- products-category
- products-id]})}
- (let [products-category [:field %products.category {:join-alias "PRODUCTS__via__PRODUCT_ID"
+ (is (partial= (mt/$ids orders
+ (merge {:source-query (let [id [:field %id {::add/source-table $$orders
+ ::add/source-alias "ID"
+ ::add/desired-alias "ID"
+ ::add/position 0}]
+ user-id [:field %user_id {::add/source-table $$orders
+ ::add/source-alias "USER_ID"
+ ::add/desired-alias "USER_ID"
+ ::add/position 1}]
+ product-id [:field %product_id {::add/source-table $$orders
+ ::add/source-alias "PRODUCT_ID"
+ ::add/desired-alias "PRODUCT_ID"
+ ::add/position 2}]
+ subtotal [:field %subtotal {::add/source-table $$orders
+ ::add/source-alias "SUBTOTAL"
+ ::add/desired-alias "SUBTOTAL"
+ ::add/position 3}]
+ tax [:field %tax {::add/source-table $$orders
+ ::add/source-alias "TAX"
+ ::add/desired-alias "TAX"
+ ::add/position 4}]
+ total [:field %total {::add/source-table $$orders
+ ::add/source-alias "TOTAL"
+ ::add/desired-alias "TOTAL"
+ ::add/position 5}]
+ discount [:field %discount {::add/source-table $$orders
+ ::add/source-alias "DISCOUNT"
+ ::add/desired-alias "DISCOUNT"
+ ::add/position 6}]
+ created-at [:field %created_at {:temporal-unit :default
+ ::add/source-table $$orders
+ ::add/source-alias "CREATED_AT"
+ ::add/desired-alias "CREATED_AT"
+ ::add/position 7}]
+ quantity [:field %quantity {::add/source-table $$orders
+ ::add/source-alias "QUANTITY"
+ ::add/desired-alias "QUANTITY"
+ ::add/position 8}]
+ pivot-grouping [:expression "pivot-grouping" {::add/desired-alias "pivot-grouping"
+ ::add/position 9}]
+ products-category [:field %products.category {:join-alias "PRODUCTS__via__PRODUCT_ID"
+ ::add/source-table "PRODUCTS__via__PRODUCT_ID"
+ ::add/source-alias "CATEGORY"
+ ::add/desired-alias "PRODUCTS__via__PRODUCT_ID__CATEGORY"
+ ::add/position 10}]
+ products-id [:field %products.id {:join-alias "PRODUCTS__via__PRODUCT_ID"
+ ::add/source-table "PRODUCTS__via__PRODUCT_ID"
+ ::add/source-alias "ID"
+ ::add/desired-alias "PRODUCTS__via__PRODUCT_ID__ID"
+ ::add/position 11}]]
+ {:source-table $$orders
+ :joins [{:source-table $$products
+ :alias "PRODUCTS__via__PRODUCT_ID"
+ :condition [:= product-id products-id]
+ :strategy :left-join
+ :fk-field-id %product_id}]
+ :expressions {"pivot-grouping" [:abs 0]}
+ :fields [id
+ user-id
+ product-id
+ subtotal
+ tax
+ total
+ discount
+ created-at
+ quantity
+ pivot-grouping
+ products-category
+ products-id]})}
+ (let [products-category [:field %products.category {:join-alias "PRODUCTS__via__PRODUCT_ID"
+ ::add/source-table ::add/source
+ ::add/source-alias "PRODUCTS__via__PRODUCT_ID__CATEGORY"
+ ::add/desired-alias "PRODUCTS__via__PRODUCT_ID__CATEGORY"
+ ::add/position 0}]
+ created-at [:field %created_at {:temporal-unit :year
+ ::nest-query/outer-select true
+ ::add/source-table ::add/source
+ ::add/source-alias "CREATED_AT"
+ ::add/desired-alias "CREATED_AT"
+ ::add/position 1}]
+ pivot-grouping [:field "pivot-grouping" {:base-type :type/Float
::add/source-table ::add/source
- ::add/source-alias "PRODUCTS__via__PRODUCT_ID__CATEGORY"
- ::add/desired-alias "PRODUCTS__via__PRODUCT_ID__CATEGORY"
- ::add/position 0}]
- created-at [:field %created_at {:temporal-unit :year
- ::nest-query/outer-select true
- ::add/source-table ::add/source
- ::add/source-alias "CREATED_AT"
- ::add/desired-alias "CREATED_AT"
- ::add/position 1}]
- pivot-grouping [:field "pivot-grouping" {:base-type :type/Float
- ::add/source-table ::add/source
- ::add/source-alias "pivot-grouping"
- ::add/desired-alias "pivot-grouping"
- ::add/position 2}]]
- {:breakout [products-category created-at pivot-grouping]
- :aggregation [[:aggregation-options [:count] {:name "count"
- ::add/desired-alias "count"
- ::add/position 3}]]
- :order-by [[:asc products-category]
- [:asc created-at]
- [:asc pivot-grouping]]})))
- (nest-expressions
- (mt/mbql-query orders
- {:aggregation [[:aggregation-options [:count] {:name "count"}]]
- :breakout [&PRODUCTS__via__PRODUCT_ID.products.category
- !year.created_at
- [:expression "pivot-grouping"]]
- :expressions {"pivot-grouping" [:abs 0]}
- :order-by [[:asc &PRODUCTS__via__PRODUCT_ID.products.category]
- [:asc !year.created_at]
- [:asc [:expression "pivot-grouping"]]]
- :joins [{:source-table $$products
- :strategy :left-join
- :alias "PRODUCTS__via__PRODUCT_ID"
- :fk-field-id %product_id
- :condition [:= $product_id &PRODUCTS__via__PRODUCT_ID.products.id]}]})))))))
+ ::add/source-alias "pivot-grouping"
+ ::add/desired-alias "pivot-grouping"
+ ::add/position 2}]]
+ {:breakout [products-category created-at pivot-grouping]
+ :aggregation [[:aggregation-options [:count] {:name "count"
+ ::add/desired-alias "count"
+ ::add/position 3}]]
+ :order-by [[:asc products-category]
+ [:asc created-at]
+ [:asc pivot-grouping]]})))
+ (nest-expressions
+ (mt/mbql-query orders
+ {:aggregation [[:aggregation-options [:count] {:name "count"}]]
+ :breakout [&PRODUCTS__via__PRODUCT_ID.products.category
+ !year.created_at
+ [:expression "pivot-grouping"]]
+ :expressions {"pivot-grouping" [:abs 0]}
+ :order-by [[:asc &PRODUCTS__via__PRODUCT_ID.products.category]
+ [:asc !year.created_at]
+ [:asc [:expression "pivot-grouping"]]]
+ :joins [{:source-table $$products
+ :strategy :left-join
+ :alias "PRODUCTS__via__PRODUCT_ID"
+ :fk-field-id %product_id
+ :condition [:= $product_id &PRODUCTS__via__PRODUCT_ID.products.id]}]})))))))
(deftest uniquify-aliases-test
(driver/with-driver :h2
(mt/dataset sample-dataset
(mt/with-everything-store
- (is (query= (mt/$ids products
- {:source-query {:source-table $$products
- :expressions {"CATEGORY" [:concat
- [:field %category {::add/source-table $$products
- ::add/source-alias "CATEGORY"
- ::add/desired-alias "CATEGORY"
- ::add/position 3}]
- "2"]}
- :fields [[:field %id {::add/source-table $$products
- ::add/source-alias "ID"
- ::add/desired-alias "ID"
- ::add/position 0}]
- [:field %ean {::add/source-table $$products
- ::add/source-alias "EAN"
- ::add/desired-alias "EAN"
- ::add/position 1}]
- [:field %title {::add/source-table $$products
- ::add/source-alias "TITLE"
- ::add/desired-alias "TITLE"
- ::add/position 2}]
- [:field %category {::add/source-table $$products
- ::add/source-alias "CATEGORY"
- ::add/desired-alias "CATEGORY"
- ::add/position 3}]
- [:field %vendor {::add/source-table $$products
- ::add/source-alias "VENDOR"
- ::add/desired-alias "VENDOR"
- ::add/position 4}]
- [:field %price {::add/source-table $$products
- ::add/source-alias "PRICE"
- ::add/desired-alias "PRICE"
- ::add/position 5}]
- [:field %rating {::add/source-table $$products
- ::add/source-alias "RATING"
- ::add/desired-alias "RATING"
- ::add/position 6}]
- [:field %created_at {:temporal-unit :default
- ::add/source-table $$products
- ::add/source-alias "CREATED_AT"
- ::add/desired-alias "CREATED_AT"
- ::add/position 7}]
- [:expression "CATEGORY" {::add/desired-alias "CATEGORY_2"
- ::add/position 8}]]}
- :breakout [[:field "CATEGORY_2" {:base-type :type/Text
- ::add/source-table ::add/source
- ::add/source-alias "CATEGORY_2"
- ::add/desired-alias "CATEGORY_2"
- ::add/position 0}]]
- :aggregation [[:aggregation-options [:count] {:name "count"
- ::add/desired-alias "count"
- ::add/position 1}]]
- :order-by [[:asc [:field "CATEGORY_2" {:base-type :type/Text
- ::add/source-table ::add/source
- ::add/source-alias "CATEGORY_2"
- ::add/desired-alias "CATEGORY_2"
- ::add/position 0}]]]
- :limit 1})
- (-> (mt/mbql-query products
- {:expressions {"CATEGORY" [:concat $category "2"]}
- :breakout [:expression"CATEGORY"]
- :aggregation [[:count]]
- :order-by [[:asc [:expression"CATEGORY"]]]
- :limit 1})
- qp/query->preprocessed
- add/add-alias-info
- :query
- nest-query/nest-expressions)))))))
+ (is (partial= (mt/$ids products
+ {:source-query {:source-table $$products
+ :expressions {"CATEGORY" [:concat
+ [:field %category {::add/source-table $$products
+ ::add/source-alias "CATEGORY"
+ ::add/desired-alias "CATEGORY"
+ ::add/position 3}]
+ "2"]}
+ :fields [[:field %id {::add/source-table $$products
+ ::add/source-alias "ID"
+ ::add/desired-alias "ID"
+ ::add/position 0}]
+ [:field %ean {::add/source-table $$products
+ ::add/source-alias "EAN"
+ ::add/desired-alias "EAN"
+ ::add/position 1}]
+ [:field %title {::add/source-table $$products
+ ::add/source-alias "TITLE"
+ ::add/desired-alias "TITLE"
+ ::add/position 2}]
+ [:field %category {::add/source-table $$products
+ ::add/source-alias "CATEGORY"
+ ::add/desired-alias "CATEGORY"
+ ::add/position 3}]
+ [:field %vendor {::add/source-table $$products
+ ::add/source-alias "VENDOR"
+ ::add/desired-alias "VENDOR"
+ ::add/position 4}]
+ [:field %price {::add/source-table $$products
+ ::add/source-alias "PRICE"
+ ::add/desired-alias "PRICE"
+ ::add/position 5}]
+ [:field %rating {::add/source-table $$products
+ ::add/source-alias "RATING"
+ ::add/desired-alias "RATING"
+ ::add/position 6}]
+ [:field %created_at {:temporal-unit :default
+ ::add/source-table $$products
+ ::add/source-alias "CREATED_AT"
+ ::add/desired-alias "CREATED_AT"
+ ::add/position 7}]
+ [:expression "CATEGORY" {::add/desired-alias "CATEGORY_2"
+ ::add/position 8}]]}
+ :breakout [[:field "CATEGORY_2" {:base-type :type/Text
+ ::add/source-table ::add/source
+ ::add/source-alias "CATEGORY_2"
+ ::add/desired-alias "CATEGORY_2"
+ ::add/position 0}]]
+ :aggregation [[:aggregation-options [:count] {:name "count"
+ ::add/desired-alias "count"
+ ::add/position 1}]]
+ :order-by [[:asc [:field "CATEGORY_2" {:base-type :type/Text
+ ::add/source-table ::add/source
+ ::add/source-alias "CATEGORY_2"
+ ::add/desired-alias "CATEGORY_2"
+ ::add/position 0}]]]
+ :limit 1})
+ (-> (mt/mbql-query products
+ {:expressions {"CATEGORY" [:concat $category "2"]}
+ :breakout [:expression"CATEGORY"]
+ :aggregation [[:count]]
+ :order-by [[:asc [:expression"CATEGORY"]]]
+ :limit 1})
+ qp/query->preprocessed
+ add/add-alias-info
+ :query
+ nest-query/nest-expressions)))))))
diff --git a/test/metabase/query_processor_test/failure_test.clj b/test/metabase/query_processor_test/failure_test.clj
index b001094d4b0..6fd872a32f4 100644
--- a/test/metabase/query_processor_test/failure_test.clj
+++ b/test/metabase/query_processor_test/failure_test.clj
@@ -7,6 +7,10 @@
[metabase.util.schema :as su]
[schema.core :as s]))
+(use-fixtures :each (fn [thunk]
+ (mt/with-log-level :fatal
+ (thunk))))
+
(defn- bad-query []
{:database (mt/id)
:type :query
@@ -24,7 +28,8 @@
:type (s/eq :query)
:query {:source-table (s/eq (mt/id :venues))
:fields (s/eq [[:field (mt/id :venues :id) {:temporal-unit :month}]])
- :limit (s/eq qp.i/absolute-max-results)}
+ :limit (s/eq qp.i/absolute-max-results)
+ s/Keyword s/Any}
(s/optional-key :driver) (s/eq :h2)
s/Keyword s/Any})
diff --git a/test/metabase/query_processor_test/query_to_native_test.clj b/test/metabase/query_processor_test/query_to_native_test.clj
index 7f291eb316a..072db411b39 100644
--- a/test/metabase/query_processor_test/query_to_native_test.clj
+++ b/test/metabase/query_processor_test/query_to_native_test.clj
@@ -5,8 +5,7 @@
[metabase.models.permissions :as perms]
[metabase.query-processor :as qp]
[metabase.test :as mt]
- [metabase.util :as u]
- [schema.core :as s]))
+ [metabase.util :as u]))
(deftest query->native-test
(testing "Can we convert an MBQL query to a native query?"
@@ -46,35 +45,28 @@
;; use `query->native`
(defn- query->native-with-user-perms
[{database-id :database, {source-table-id :source-table} :query, :as query} {:keys [object-perms? native-perms?]}]
- (try
- (binding [api/*current-user-id* Integer/MAX_VALUE
- api/*current-user-permissions-set* (delay (cond-> #{}
- object-perms? (conj (perms/data-perms-path database-id "PUBLIC" source-table-id))
- native-perms? (conj (perms/adhoc-native-query-path database-id))))]
- (qp/query->native query))
- (catch clojure.lang.ExceptionInfo e
- (merge {:error (.getMessage e)}
- (ex-data e)))))
+ (binding [api/*current-user-id* Integer/MAX_VALUE
+ api/*current-user-permissions-set* (delay (cond-> #{}
+ object-perms? (conj (perms/data-perms-path database-id "PUBLIC" source-table-id))
+ native-perms? (conj (perms/adhoc-native-query-path database-id))))]
+ (qp/query->native query)))
(deftest permissions-test
- (testing "If user permissions are bound, we should do permissions checking when you call `query->native`"
+ (testing "If user permissions are bound, we should still NOT do permissions checking when you call `query->native`"
(testing "Should work if you have the right perms"
- (is (= true
- (boolean
- (query->native-with-user-perms
- (mt/mbql-query venues)
- {:object-perms? true, :native-perms? true})))))
- (testing "If you don't have MBQL permissions for the original query it should throw an error"
- (is (schema= {:error (s/eq "You do not have permissions to run this query.")
- s/Any s/Any}
- (query->native-with-user-perms
- (mt/mbql-query venues)
- {:object-perms? false, :native-perms? true}))))))
+ (is (query->native-with-user-perms
+ (mt/mbql-query venues)
+ {:object-perms? true, :native-perms? true})))
+ (testing "Should still work even WITHOUT the right perms"
+ (is (query->native-with-user-perms
+ (mt/mbql-query venues)
+ {:object-perms? false, :native-perms? true})))))
(deftest error-test
(testing "If the query is bad in some way it should return a relevant error (?)"
- (is (schema= {:error (s/eq (format "Database %d does not exist." Integer/MAX_VALUE))
- s/Any s/Any}
- (query->native-with-user-perms
- {:database Integer/MAX_VALUE, :type :query, :query {:source-table Integer/MAX_VALUE}}
- {:object-perms? true, :native-perms? true})))))
+ (is (thrown-with-msg?
+ clojure.lang.ExceptionInfo
+ #"Database \d+ does not exist"
+ (query->native-with-user-perms
+ {:database Integer/MAX_VALUE, :type :query, :query {:source-table Integer/MAX_VALUE}}
+ {:object-perms? true, :native-perms? true})))))
--
GitLab