diff --git a/src/metabase/query_processor/macros.clj b/src/metabase/query_processor/macros.clj
deleted file mode 100644
index 6d1016528ff586871c37b5ac5e451c2aee6772ee..0000000000000000000000000000000000000000
--- a/src/metabase/query_processor/macros.clj
+++ /dev/null
@@ -1,117 +0,0 @@
-(ns metabase.query-processor.macros
- "Code in charge of expanding [\"METRIC\" ...] and [\"SEGMENT\" ...] forms in MBQL queries.
- (METRIC forms are expanded into aggregations and sometimes filter clauses, while SEGMENT forms
- are expanded into filter clauses.)
-
- TODO - this namespace is ancient and written with MBQL '95 in mind, e.g. it is case-sensitive.
- At some point this ought to be reworked to be case-insensitive and cleaned up.
-
- TODO - The actual middleware that applies these functions lives in `metabase.query-processor.middleware.expand-macros`.
- Not sure those two namespaces need to be divided. We can probably move all the functions in this namespace into that
- one; that might require shuffling around some tests as well."
- (:require [clojure.core.match :refer [match]]
- [clojure.walk :as walk]
- [toucan.db :as db]))
-
-(defn- non-empty-clause? [clause]
- (and clause
- (or (not (sequential? clause))
- (and (seq clause)
- (not (every? nil? clause))))))
-
-;;; ------------------------------------------------------------ Segments ------------------------------------------------------------
-
-(defn- segment-parse-filter-subclause [form]
- (when (non-empty-clause? form)
- (match form
- ["SEGMENT" (segment-id :guard integer?)] (:filter (db/select-one-field :definition 'Segment :id segment-id))
- subclause subclause
- form (throw (java.lang.Exception. (format "segment-parse-filter-subclause failed: invalid clause: %s" form))))))
-
-(defn- segment-parse-filter [form]
- (when (non-empty-clause? form)
- (match form
- ["AND" & subclauses] (into ["AND"] (mapv segment-parse-filter subclauses))
- ["OR" & subclauses] (into ["OR"] (mapv segment-parse-filter subclauses))
- subclause (segment-parse-filter-subclause subclause)
- form (throw (java.lang.Exception. (format "segment-parse-filter failed: invalid clause: %s" form))))))
-
-(defn- macroexpand-segment [query-dict]
- (if (non-empty-clause? (get-in query-dict [:query :filter]))
- (update-in query-dict [:query :filter] segment-parse-filter)
- query-dict))
-
-(defn- merge-filter-clauses [base addtl]
- (cond
- (and (seq base)
- (seq addtl)) ["AND" base addtl]
- (seq base) base
- (seq addtl) addtl
- :else []))
-
-
-;;; ------------------------------------------------------------ Metrics ------------------------------------------------------------
-
-(defn- metric? [aggregation]
- (match aggregation
- ["METRIC" (_ :guard integer?)] true
- _ false))
-
-(defn- metric-id [metric]
- (when (metric? metric)
- (second metric)))
-
-(defn- maybe-unnest-ag-clause
- "Unnest AG-CLAUSE if it's wrapped in a vector (i.e. if it is using the \"multiple-aggregation\" syntax).
- (This is provided merely as a convenience to facilitate implementation of the Query Builder, so it can use the same UI for
- normal aggregations and Metric creation. *METRICS DO NOT SUPPORT MULTIPLE AGGREGATIONS,* so if nested syntax is used, any
- aggregation after the first will be ignored.)"
- [ag-clause]
- (if (and (coll? ag-clause)
- (every? coll? ag-clause))
- (first ag-clause)
- ag-clause))
-
-(defn- expand-metric [metric-clause filter-clauses-atom]
- (let [{filter-clause :filter, ag-clause :aggregation} (db/select-one-field :definition 'Metric, :id (metric-id metric-clause))]
- (when filter-clause
- (swap! filter-clauses-atom conj filter-clause))
- (maybe-unnest-ag-clause ag-clause)))
-
-(defn- expand-metrics-in-ag-clause [query-dict filter-clauses-atom]
- (walk/postwalk (fn [form]
- (if-not (metric? form)
- form
- (expand-metric form filter-clauses-atom)))
- query-dict))
-
-(defn- add-metrics-filter-clauses
- "Add any FILTER-CLAUSES to the QUERY-DICT. If query has existing filter clauses, the new ones are
- combined with an `:and` filter clause."
- [query-dict filter-clauses]
- (if-not (seq filter-clauses)
- query-dict
- (update-in query-dict [:query :filter] merge-filter-clauses (if (> (count filter-clauses) 1)
- (cons "AND" filter-clauses)
- (first filter-clauses)))))
-
-(defn- expand-metrics [query-dict]
- (let [filter-clauses-atom (atom [])
- query-dict (expand-metrics-in-ag-clause query-dict filter-clauses-atom)]
- (add-metrics-filter-clauses query-dict @filter-clauses-atom)))
-
-(defn- macroexpand-metric [{{aggregations :aggregation} :query, :as query-dict}]
- (if-not (seq aggregations)
- ;; :aggregation is empty, so no METRIC to expand
- query-dict
- ;; otherwise walk the query dict and expand METRIC clauses
- (expand-metrics query-dict)))
-
-
-;;; ------------------------------------------------------------ Middleware ------------------------------------------------------------
-
-(defn expand-macros "Expand the macros (SEGMENT, METRIC) in a QUERY-DICT."
- [query-dict]
- (-> query-dict
- macroexpand-metric
- macroexpand-segment))
diff --git a/src/metabase/query_processor/middleware/expand_macros.clj b/src/metabase/query_processor/middleware/expand_macros.clj
index f8f89df8653897c580696e08ae4c669d211853d7..039b95275cbcd68123970c5422d97ec70990f718 100644
--- a/src/metabase/query_processor/middleware/expand_macros.clj
+++ b/src/metabase/query_processor/middleware/expand_macros.clj
@@ -1,15 +1,131 @@
(ns metabase.query-processor.middleware.expand-macros
- "Middleware for expanding `METRIC` and `SEGMENT` 'macros' in *unexpanded* MBQL queries."
+ "Middleware for expanding `METRIC` and `SEGMENT` 'macros' in *unexpanded* MBQL queries.
+
+ Code in charge of expanding [\"METRIC\" ...] and [\"SEGMENT\" ...] forms in MBQL queries.
+ (METRIC forms are expanded into aggregations and sometimes filter clauses, while SEGMENT forms
+ are expanded into filter clauses.)
+
+ TODO - this namespace is ancient and written with MBQL '95 in mind, e.g. it is case-sensitive.
+ At some point this ought to be reworked to be case-insensitive and cleaned up."
(:require [clojure.tools.logging :as log]
(metabase.query-processor [interface :as i]
- [macros :as macros]
[util :as qputil])
+ [clojure.core.match :refer [match]]
+ [clojure.walk :as walk]
+ [toucan.db :as db]
[metabase.util :as u]))
+
+;;; ------------------------------------------------------------ Utils ------------------------------------------------------------
+
+(defn- non-empty-clause? [clause]
+ (and clause
+ (or (not (sequential? clause))
+ (and (seq clause)
+ (not (every? nil? clause))))))
+
+;;; ------------------------------------------------------------ Segments ------------------------------------------------------------
+
+(defn- segment-parse-filter-subclause [form]
+ (when (non-empty-clause? form)
+ (match form
+ ["SEGMENT" (segment-id :guard integer?)] (:filter (db/select-one-field :definition 'Segment :id segment-id))
+ subclause subclause
+ form (throw (java.lang.Exception. (format "segment-parse-filter-subclause failed: invalid clause: %s" form))))))
+
+(defn- segment-parse-filter [form]
+ (when (non-empty-clause? form)
+ (match form
+ ["AND" & subclauses] (into ["AND"] (mapv segment-parse-filter subclauses))
+ ["OR" & subclauses] (into ["OR"] (mapv segment-parse-filter subclauses))
+ subclause (segment-parse-filter-subclause subclause)
+ form (throw (java.lang.Exception. (format "segment-parse-filter failed: invalid clause: %s" form))))))
+
+(defn- expand-segments [query-dict]
+ (if (non-empty-clause? (get-in query-dict [:query :filter]))
+ (update-in query-dict [:query :filter] segment-parse-filter)
+ query-dict))
+
+(defn- merge-filter-clauses [base addtl]
+ (cond
+ (and (seq base)
+ (seq addtl)) ["AND" base addtl]
+ (seq base) base
+ (seq addtl) addtl
+ :else []))
+
+
+;;; ------------------------------------------------------------ Metrics ------------------------------------------------------------
+
+(defn- metric? [aggregation]
+ (match aggregation
+ ["METRIC" (_ :guard integer?)] true
+ _ false))
+
+(defn- metric-id [metric]
+ (when (metric? metric)
+ (second metric)))
+
+(defn- maybe-unnest-ag-clause
+ "Unnest AG-CLAUSE if it's wrapped in a vector (i.e. if it is using the \"multiple-aggregation\" syntax).
+ (This is provided merely as a convenience to facilitate implementation of the Query Builder, so it can use the same UI for
+ normal aggregations and Metric creation. *METRICS DO NOT SUPPORT MULTIPLE AGGREGATIONS,* so if nested syntax is used, any
+ aggregation after the first will be ignored.)"
+ [ag-clause]
+ (if (and (coll? ag-clause)
+ (every? coll? ag-clause))
+ (first ag-clause)
+ ag-clause))
+
+(defn- expand-metric [metric-clause filter-clauses-atom]
+ (let [{filter-clause :filter, ag-clause :aggregation} (db/select-one-field :definition 'Metric, :id (metric-id metric-clause))]
+ (when filter-clause
+ (swap! filter-clauses-atom conj filter-clause))
+ (maybe-unnest-ag-clause ag-clause)))
+
+(defn- expand-metrics-in-ag-clause [query-dict filter-clauses-atom]
+ (walk/postwalk (fn [form]
+ (if-not (metric? form)
+ form
+ (expand-metric form filter-clauses-atom)))
+ query-dict))
+
+(defn- add-metrics-filter-clauses
+ "Add any FILTER-CLAUSES to the QUERY-DICT. If query has existing filter clauses, the new ones are
+ combined with an `:and` filter clause."
+ [query-dict filter-clauses]
+ (if-not (seq filter-clauses)
+ query-dict
+ (update-in query-dict [:query :filter] merge-filter-clauses (if (> (count filter-clauses) 1)
+ (cons "AND" filter-clauses)
+ (first filter-clauses)))))
+
+(defn- expand-metrics* [query-dict]
+ (let [filter-clauses-atom (atom [])
+ query-dict (expand-metrics-in-ag-clause query-dict filter-clauses-atom)]
+ (add-metrics-filter-clauses query-dict @filter-clauses-atom)))
+
+(defn- expand-metrics [{{aggregations :aggregation} :query, :as query-dict}]
+ (if-not (seq aggregations)
+ ;; :aggregation is empty, so no METRIC to expand
+ query-dict
+ ;; otherwise walk the query dict and expand METRIC clauses
+ (expand-metrics* query-dict)))
+
+
+;;; ------------------------------------------------------------ Middleware ------------------------------------------------------------
+
+(defn- expand-metrics-and-segments "Expand the macros (SEGMENT, METRIC) in a QUERY."
+ [query]
+ (-> query
+ expand-metrics
+ expand-segments))
+
+
(defn- expand-macros* [query]
(if-not (qputil/mbql-query? query)
query
- (u/prog1 (macros/expand-macros query)
+ (u/prog1 (expand-metrics-and-segments query)
(when (and (not i/*disable-qp-logging*)
(not= <> query))
(log/debug (u/format-color 'cyan "\n\nMACRO/SUBSTITUTED: %s\n%s" (u/emoji "😻") (u/pprint-to-str <>)))))))
diff --git a/test/metabase/query_processor/macros_test.clj b/test/metabase/query_processor/middleware/expand_macros_test.clj
similarity index 63%
rename from test/metabase/query_processor/macros_test.clj
rename to test/metabase/query_processor/middleware/expand_macros_test.clj
index dcfa8e281b4fb92765d59ab93a704984ed2cb500..c98831e5a0f682cc3a879922a1c1b84b4dba56cb 100644
--- a/test/metabase/query_processor/macros_test.clj
+++ b/test/metabase/query_processor/middleware/expand_macros_test.clj
@@ -1,4 +1,4 @@
-(ns metabase.query-processor.macros-test
+(ns metabase.query-processor.middleware.expand-macros-test
(:require [expectations :refer :all]
[metabase
[query-processor :as qp]
@@ -9,14 +9,16 @@
[metric :refer [Metric]]
[segment :refer [Segment]]
[table :refer [Table]]]
- [metabase.query-processor
- [expand :as ql]
- [macros :refer :all]]
- [metabase.test.data :as data]
+ [metabase.query-processor.expand :as ql]
+ [metabase.query-processor.middleware.expand-macros :refer :all]
+ [metabase.test
+ [data :as data]
+ [util :as tu]]
[metabase.test.data.datasets :as datasets]
[toucan.util.test :as tt]))
-;; expand-macros
+;; expand-metrics-and-segments
+(tu/resolve-private-vars metabase.query-processor.middleware.expand-macros expand-metrics-and-segments)
;; no Segment or Metric should yield exact same query
(expect
@@ -25,11 +27,11 @@
:query {:aggregation ["rows"]
:filter ["AND" [">" 4 1]]
:breakout [17]}}
- (expand-macros {:database 1
- :type :query
- :query {:aggregation ["rows"]
- :filter ["AND" [">" 4 1]]
- :breakout [17]}}))
+ (expand-metrics-and-segments {:database 1
+ :type :query
+ :query {:aggregation ["rows"]
+ :filter ["AND" [">" 4 1]]
+ :breakout [17]}}))
;; just segments
(expect
@@ -37,8 +39,8 @@
:type :query
:query {:aggregation ["rows"]
:filter ["AND" ["AND" ["=" 5 "abc"]]
- ["OR" ["AND" ["IS_NULL" 7]]
- [">" 4 1]]]
+ ["OR" ["AND" ["IS_NULL" 7]]
+ [">" 4 1]]]
:breakout [17]}}
(tt/with-temp* [Database [{database-id :id}]
Table [{table-id :id} {:db_id database-id}]
@@ -46,11 +48,11 @@
:definition {:filter ["AND" ["=" 5 "abc"]]}}]
Segment [{segment-2-id :id} {:table_id table-id
:definition {:filter ["AND" ["IS_NULL" 7]]}}]]
- (expand-macros {:database 1
- :type :query
- :query {:aggregation ["rows"]
- :filter ["AND" ["SEGMENT" segment-1-id] ["OR" ["SEGMENT" segment-2-id] [">" 4 1]]]
- :breakout [17]}})))
+ (expand-metrics-and-segments {:database 1
+ :type :query
+ :query {:aggregation ["rows"]
+ :filter ["AND" ["SEGMENT" segment-1-id] ["OR" ["SEGMENT" segment-2-id] [">" 4 1]]]
+ :breakout [17]}})))
;; just a metric (w/out nested segments)
(expect
@@ -58,7 +60,7 @@
:type :query
:query {:aggregation ["count"]
:filter ["AND" ["AND" [">" 4 1]]
- ["AND" ["=" 5 "abc"]]]
+ ["AND" ["=" 5 "abc"]]]
:breakout [17]
:order_by [[1 "ASC"]]}}
(tt/with-temp* [Database [{database-id :id}]
@@ -66,12 +68,12 @@
Metric [{metric-1-id :id} {:table_id table-id
:definition {:aggregation ["count"]
:filter ["AND" ["=" 5 "abc"]]}}]]
- (expand-macros {:database 1
- :type :query
- :query {:aggregation ["METRIC" metric-1-id]
- :filter ["AND" [">" 4 1]]
- :breakout [17]
- :order_by [[1 "ASC"]]}})))
+ (expand-metrics-and-segments {:database 1
+ :type :query
+ :query {:aggregation ["METRIC" metric-1-id]
+ :filter ["AND" [">" 4 1]]
+ :breakout [17]
+ :order_by [[1 "ASC"]]}})))
;; check that when the original filter is empty we simply use our metric filter definition instead
(expect
@@ -86,12 +88,12 @@
Metric [{metric-1-id :id} {:table_id table-id
:definition {:aggregation ["count"]
:filter ["AND" ["=" 5 "abc"]]}}]]
- (expand-macros {:database 1
- :type :query
- :query {:aggregation ["METRIC" metric-1-id]
- :filter []
- :breakout [17]
- :order_by [[1 "ASC"]]}})))
+ (expand-metrics-and-segments {:database 1
+ :type :query
+ :query {:aggregation ["METRIC" metric-1-id]
+ :filter []
+ :breakout [17]
+ :order_by [[1 "ASC"]]}})))
;; metric w/ no filter definition
(expect
@@ -105,12 +107,12 @@
Table [{table-id :id} {:db_id database-id}]
Metric [{metric-1-id :id} {:table_id table-id
:definition {:aggregation ["count"]}}]]
- (expand-macros {:database 1
- :type :query
- :query {:aggregation ["METRIC" metric-1-id]
- :filter ["AND" ["=" 5 "abc"]]
- :breakout [17]
- :order_by [[1 "ASC"]]}})))
+ (expand-metrics-and-segments {:database 1
+ :type :query
+ :query {:aggregation ["METRIC" metric-1-id]
+ :filter ["AND" ["=" 5 "abc"]]
+ :breakout [17]
+ :order_by [[1 "ASC"]]}})))
;; a metric w/ nested segments
(expect
@@ -129,12 +131,12 @@
Metric [{metric-1-id :id} {:table_id table-id
:definition {:aggregation ["sum" 18]
:filter ["AND" ["=" 5 "abc"] ["SEGMENT" segment-1-id]]}}]]
- (expand-macros {:database 1
- :type :query
- :query {:aggregation ["METRIC" metric-1-id]
- :filter ["AND" [">" 4 1] ["SEGMENT" segment-2-id]]
- :breakout [17]
- :order_by [[1 "ASC"]]}})))
+ (expand-metrics-and-segments {:database 1
+ :type :query
+ :query {:aggregation ["METRIC" metric-1-id]
+ :filter ["AND" [">" 4 1] ["SEGMENT" segment-2-id]]
+ :breakout [17]
+ :order_by [[1 "ASC"]]}})))
;; Check that a metric w/ multiple aggregation syntax (nested vector) still works correctly
(datasets/expect-with-engines (engines-that-support :expression-aggregations)