diff --git a/src/metabase/driver/druid/query_processor.clj b/src/metabase/driver/druid/query_processor.clj
index 14eb7067143b50ea75f84251af9f7a0e81ba9fa6..c617e1a7b7328c02ffba3c49237d8187a4ea2e4b 100644
--- a/src/metabase/driver/druid/query_processor.clj
+++ b/src/metabase/driver/druid/query_processor.clj
@@ -218,7 +218,7 @@
(ag:count output-name))))
-(defn- handle-aggregation [query-type {ag-type :aggregation-type, ag-field :field, output-name :output-name, custom-name :name, :as ag} druid-query]
+(defn- handle-aggregation [query-type {ag-type :aggregation-type, ag-field :field, output-name :output-name, custom-name :custom-name, :as ag} druid-query]
(let [output-name (or custom-name output-name)]
(when (isa? query-type ::ag-query)
(merge-with concat
diff --git a/src/metabase/query_processor/annotate.clj b/src/metabase/query_processor/annotate.clj
index f373bf7ebdf033e7bba5f8569fc5784adcbc3c43..fe6e778d5d501001083edeadd71489478413ff18 100644
--- a/src/metabase/query_processor/annotate.clj
+++ b/src/metabase/query_processor/annotate.clj
@@ -94,10 +94,10 @@
:count
ag-type))
-;; TODO - rename to `aggregation-name` now that this handles any sort of aggregation
+;; TODO - rename to something like `aggregation-name` or `aggregation-subclause-name` now that this handles any sort of aggregation
(defn expression-aggregation-name
- "Return an appropriate name for an AGGREGATION, e.g. `sum + count`."
- ^String [{custom-name :name, aggregation-type :aggregation-type, :as ag}]
+ "Return an appropriate name for an `:aggregation` subclause (an aggregation or expression)."
+ ^String [{custom-name :custom-name, aggregation-type :aggregation-type, :as ag}]
(cond
;; if a custom name was provided use it
custom-name custom-name
diff --git a/src/metabase/query_processor/expand.clj b/src/metabase/query_processor/expand.clj
index 823ae6dcaed427a52999c2ab7ccef94fb0826ce5..7c08ac74c45b439df87b418962363aa787aff1ed 100644
--- a/src/metabase/query_processor/expand.clj
+++ b/src/metabase/query_processor/expand.clj
@@ -64,7 +64,7 @@
(This will probably be extended to support Fields in the future, but for now, only the `:aggregation` clause is supported.)"
{:added "0.22.0"}
[aggregation-or-expression :- i/Aggregation, custom-name :- su/NonBlankString]
- (assoc aggregation-or-expression :name custom-name))
+ (assoc aggregation-or-expression :custom-name custom-name))
(s/defn ^:ql ^:always-validate datetime-field :- FieldPlaceholder
"Reference to a `DateTimeField`. This is just a `Field` reference with an associated datetime UNIT."
diff --git a/src/metabase/query_processor/interface.clj b/src/metabase/query_processor/interface.clj
index b1a15f6f958b3517a4d98ee0a5cd5ebcf2e15c31..526e887800f025b620fb5d0c0d7102f16fd88bba 100644
--- a/src/metabase/query_processor/interface.clj
+++ b/src/metabase/query_processor/interface.clj
@@ -181,10 +181,10 @@
(def ^:private ExpressionOperator (s/named (s/enum :+ :- :* :/) "Valid expression operator"))
-(s/defrecord Expression [operator :- ExpressionOperator
- args :- [(s/cond-pre (s/recursive #'RValue)
- (s/recursive #'Aggregation))]
- name :- (s/maybe su/NonBlankString)])
+(s/defrecord Expression [operator :- ExpressionOperator
+ args :- [(s/cond-pre (s/recursive #'RValue)
+ (s/recursive #'Aggregation))]
+ custom-name :- (s/maybe su/NonBlankString)])
(def AnyFieldOrExpression
"Schema for a `FieldPlaceholder`, `AgRef`, or `Expression`."
@@ -243,13 +243,13 @@
(s/defrecord AggregationWithoutField [aggregation-type :- (s/named (s/enum :count :cumulative-count)
"Valid aggregation type")
- name :- (s/maybe su/NonBlankString)])
+ custom-name :- (s/maybe su/NonBlankString)])
(s/defrecord AggregationWithField [aggregation-type :- (s/named (s/enum :avg :count :cumulative-sum :distinct :max :min :stddev :sum)
"Valid aggregation type")
field :- (s/cond-pre FieldPlaceholderOrExpressionRef
Expression)
- name :- (s/maybe su/NonBlankString)])
+ custom-name :- (s/maybe su/NonBlankString)])
(defn- valid-aggregation-for-driver? [{:keys [aggregation-type]}]
(when (= aggregation-type :stddev)
diff --git a/src/metabase/query_processor/macros.clj b/src/metabase/query_processor/macros.clj
index d567c110544b4069ea65e26dd87da2c34e6458ad..dfe291fc13f9d56804b35e9925a8d6872564464e 100644
--- a/src/metabase/query_processor/macros.clj
+++ b/src/metabase/query_processor/macros.clj
@@ -71,10 +71,15 @@
(expand-metric form filter-clauses-atom)))
query-dict))
+(defn- add-metrics-filter-clauses [query-dict filter-clauses]
+ (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)]
- (update-in query-dict [:query :filter] merge-filter-clauses @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)
diff --git a/test/metabase/api/dataset_test.clj b/test/metabase/api/dataset_test.clj
index 24d491c85f756657fcdec68657272bd3e7eb07d5..c6f2459457da028576d2a26ab9d4dea8fac4e6b8 100644
--- a/test/metabase/api/dataset_test.clj
+++ b/test/metabase/api/dataset_test.clj
@@ -64,7 +64,7 @@
(query checkins
(ql/aggregation (ql/count))))
(assoc :type "query")
- (assoc-in [:query :aggregation] [{:aggregation-type "count"}])
+ (assoc-in [:query :aggregation] [{:aggregation-type "count", :custom-name nil}])
(assoc :constraints query-constraints))
:started_at true
:finished_at true
@@ -80,7 +80,7 @@
(query checkins
(ql/aggregation (ql/count))))
(assoc :type "query")
- (assoc-in [:query :aggregation] [{:aggregation-type "count"}])
+ (assoc-in [:query :aggregation] [{:aggregation-type "count", :custom-name nil}])
(assoc :constraints query-constraints))
:started_at true
:finished_at true
diff --git a/test/metabase/events/activity_feed_test.clj b/test/metabase/events/activity_feed_test.clj
index 70231b803d729694b16aadcfdefc0775b40c2a65..0a541ffe77216282fff7e8273da2267b66a1746a 100644
--- a/test/metabase/events/activity_feed_test.clj
+++ b/test/metabase/events/activity_feed_test.clj
@@ -271,7 +271,7 @@
:model "segment"
:model_id (:id segment)
:database_id (id)
- :table_id (id :venues)
+ :table_id (id :checkins)
:details {:name (:name segment)
:description (:description segment)}}
(with-temp-activities
@@ -288,7 +288,7 @@
:model "segment"
:model_id (:id segment)
:database_id (id)
- :table_id (id :venues)
+ :table_id (id :checkins)
:details {:name (:name segment)
:description (:description segment)
:revision_message "update this mofo"}}
@@ -310,7 +310,7 @@
:model "segment"
:model_id (:id segment)
:database_id (id)
- :table_id (id :venues)
+ :table_id (id :checkins)
:details {:name (:name segment)
:description (:description segment)
:revision_message "deleted"}}
diff --git a/test/metabase/query_processor/expand_resolve_test.clj b/test/metabase/query_processor/expand_resolve_test.clj
index 8b7381575bf50db5fdb41bba0b15da97757df89c..cb381f32c1628d74451e7952b93522aee0c199ef 100644
--- a/test/metabase/query_processor/expand_resolve_test.clj
+++ b/test/metabase/query_processor/expand_resolve_test.clj
@@ -222,9 +222,10 @@
:type :query
:query {:source-table (id :checkins)
:aggregation [{:aggregation-type :sum
- :field {:field-id (id :venues :price)
- :fk-field-id (id :checkins :venue_id)
- :datetime-unit nil}}]
+ :custom-name nil
+ :field {:field-id (id :venues :price)
+ :fk-field-id (id :checkins :venue_id)
+ :datetime-unit nil}}]
:breakout [{:field-id (id :checkins :date)
:fk-field-id nil
:datetime-unit :day-of-week}]}}
@@ -235,20 +236,21 @@
:name "CHECKINS"
:id (id :checkins)}
:aggregation [{:aggregation-type :sum
- :field {:description nil
- :base-type :type/Integer
- :parent nil
- :table-id (id :venues)
- :special-type :type/Category
- :field-name "PRICE"
- :field-display-name "Price"
- :parent-id nil
- :visibility-type :normal
- :position nil
- :field-id (id :venues :price)
- :fk-field-id (id :checkins :venue_id)
- :table-name "VENUES__via__VENUE_ID"
- :schema-name nil}}]
+ :custom-name nil
+ :field {:description nil
+ :base-type :type/Integer
+ :parent nil
+ :table-id (id :venues)
+ :special-type :type/Category
+ :field-name "PRICE"
+ :field-display-name "Price"
+ :parent-id nil
+ :visibility-type :normal
+ :position nil
+ :field-id (id :venues :price)
+ :fk-field-id (id :checkins :venue_id)
+ :table-name "VENUES__via__VENUE_ID"
+ :schema-name nil}}]
:breakout [{:field {:description nil
:base-type :type/Date
:parent nil
@@ -275,7 +277,7 @@
:fk-field-ids #{(id :checkins :venue_id)}
:table-ids #{(id :venues) (id :checkins)}}]
(let [expanded-form (ql/expand (wrap-inner-query (query checkins
- (ql/aggregation (ql/sum $venue_id->venues.price))
- (ql/breakout (ql/datetime-field $checkins.date :day-of-week)))))]
+ (ql/aggregation (ql/sum $venue_id->venues.price))
+ (ql/breakout (ql/datetime-field $checkins.date :day-of-week)))))]
(mapv obj->map [expanded-form
(resolve/resolve expanded-form)])))
diff --git a/test/metabase/query_processor/macros_test.clj b/test/metabase/query_processor/macros_test.clj
index 8cc3bf487dbd0a7dcf0daff9ca7ad809ba6434a2..5c39a816075be96050860a83775e4ef56fa3a643 100644
--- a/test/metabase/query_processor/macros_test.clj
+++ b/test/metabase/query_processor/macros_test.clj
@@ -14,7 +14,7 @@
(expect
{:database 1
:type :query
- :query {:aggregation [["rows"]]
+ :query {:aggregation ["rows"]
:filter ["AND" [">" 4 1]]
:breakout [17]}}
(expand-macros {:database 1
@@ -27,8 +27,10 @@
(expect
{:database 1
:type :query
- :query {:aggregation [["rows"]]
- :filter ["AND" ["AND" ["=" 5 "abc"]] ["OR" ["AND" ["IS_NULL" 7]] [">" 4 1]]]
+ :query {:aggregation ["rows"]
+ :filter ["AND" ["AND" ["=" 5 "abc"]]
+ ["OR" ["AND" ["IS_NULL" 7]]
+ [">" 4 1]]]
:breakout [17]}}
(tu/with-temp* [Database [{database-id :id}]
Table [{table-id :id} {:db_id database-id}]
@@ -46,8 +48,9 @@
(expect
{:database 1
:type :query
- :query {:aggregation [["count"]]
- :filter ["AND" ["AND" [">" 4 1]] ["AND" ["=" 5 "abc"]]]
+ :query {:aggregation ["count"]
+ :filter ["AND" ["AND" [">" 4 1]]
+ ["AND" ["=" 5 "abc"]]]
:breakout [17]
:order_by [[1 "ASC"]]}}
(tu/with-temp* [Database [{database-id :id}]
@@ -66,7 +69,7 @@
(expect
{:database 1
:type :query
- :query {:aggregation [["count"]]
+ :query {:aggregation ["count"]
:filter ["AND" ["=" 5 "abc"]]
:breakout [17]
:order_by [[1 "ASC"]]}}
@@ -86,7 +89,7 @@
(expect
{:database 1
:type :query
- :query {:aggregation [["count"]]
+ :query {:aggregation ["count"]
:filter ["AND" ["=" 5 "abc"]]
:breakout [17]
:order_by [[1 "ASC"]]}}
@@ -105,7 +108,7 @@
(expect
{:database 1
:type :query
- :query {:aggregation [["sum" 18]]
+ :query {:aggregation ["sum" 18]
:filter ["AND" ["AND" [">" 4 1] ["AND" ["IS_NULL" 7]]] ["AND" ["=" 5 "abc"] ["AND" ["BETWEEN" 9 0 25]]]]
:breakout [17]
:order_by [[1 "ASC"]]}}
diff --git a/test/metabase/query_processor_test/expression_aggregations_test.clj b/test/metabase/query_processor_test/expression_aggregations_test.clj
index 5b6452442b8958ed195ae18479c7867b87c33dc3..d69018fe9d9f913f15b9441ec4e7245f3e083951 100644
--- a/test/metabase/query_processor_test/expression_aggregations_test.clj
+++ b/test/metabase/query_processor_test/expression_aggregations_test.clj
@@ -167,7 +167,7 @@
[3 52]
[4 30]]
:columns [(data/format-name "price")
- "New Price"]}
+ (if (= *engine* :redshift) "new price" "New Price")]} ; Redshift annoyingly always lowercases column aliases
(format-rows-by [int int]
(rows+column-names (data/run-query venues
(ql/aggregation (ql/named (ql/sum (ql/+ $price 1)) "New Price"))
@@ -180,7 +180,7 @@
[3 -2]
[4 -17]]
:columns [(data/format-name "price")
- "Sum-41"]}
+ (if (= *engine* :redshift) "sum-41" "Sum-41")]}
(format-rows-by [int int]
(rows+column-names (data/run-query venues
(ql/aggregation (ql/named (ql/- (ql/sum $price) 41) "Sum-41"))