Ensure aggregation metadata has effective-type (#31100)

src/metabase/lib/aggregation.cljc

@@ -237,9 +237,11 @@
     stage-number :- :int]
    (some->> (not-empty (:aggregation (lib.util/query-stage query stage-number)))
             (into [] (map (fn [aggregation]
-                            (-> (lib.metadata.calculation/metadata query stage-number aggregation)
-                                (assoc :lib/source :source/aggregations
-                                       :lib/source-uuid (:lib/uuid (second aggregation))))))))))
+                            (let [metadata (lib.metadata.calculation/metadata query stage-number aggregation)]
+                              (-> metadata
+                                  (u/assoc-default :effective-type (or (:base-type metadata) :type/*))
+                                  (assoc :lib/source :source/aggregations
+                                         :lib/source-uuid (:lib/uuid (second aggregation)))))))))))
 
 (def ^:private OperatorWithColumns
   [:merge

src/metabase/lib/stage.cljc

@@ -16,6 +16,7 @@
    [metabase.lib.schema.id :as lib.schema.id]
    [metabase.lib.util :as lib.util]
    [metabase.shared.util.i18n :as i18n]
+   [metabase.util :as u]
    [metabase.util.malli :as mu]))
 
 (declare stage-metadata)
@@ -155,13 +156,11 @@
   (not-empty
    (for [expression (lib.expression/expressions-metadata query stage-number)]
      (let [base-type (:base-type expression)]
-       (cond-> (assoc expression
-                      :lib/source               :source/expressions
-                      :lib/source-column-alias  (:name expression)
-                      :lib/desired-column-alias (unique-name-fn (:name expression)))
-         (and (not (:effective-type expression))
-              base-type)
-         (assoc :effective-type base-type))))))
+       (-> (assoc expression
+                  :lib/source               :source/expressions
+                  :lib/source-column-alias  (:name expression)
+                  :lib/desired-column-alias (unique-name-fn (:name expression)))
+           (u/assoc-default :effective-type (or base-type :type/*)))))))
 
 ;;; Calculate the columns to return if `:aggregations`/`:breakout`/`:fields` are unspecified.
 ;;;

src/metabase/util.cljc

@@ -784,3 +784,11 @@
   (if (some? v)
     (assoc m k v)
     (dissoc m k)))
+
+(defn assoc-default
+  "Called like `(assoc m k v)`, this does [[assoc]] iff `m` does not contain `k`
+  and `v` is not nil."
+  [m k v]
+  (if (or (nil? v) (contains? m k))
+    m
+    (assoc m k v)))

test/metabase/lib/aggregation_test.cljc

@@ -10,6 +10,7 @@
    [metabase.lib.schema.expression :as lib.schema.expression]
    [metabase.lib.test-metadata :as meta]
    [metabase.lib.test-util :as lib.tu]
+   [metabase.lib.types.isa :as lib.types.isa]
    [metabase.lib.util :as lib.util]))
 
 #?(:cljs (comment metabase.test-runner.assert-exprs.approximately-equal/keep-me))
@@ -404,22 +405,22 @@
                     [:count {}]
                     [:sum {} [:field {:base-type :type/Integer, :effective-type :type/Integer} int?]]]}]}
                 agg-query))
-        (is (=? [{:lib/type     :metadata/field,
-                  :base-type    :type/Integer,
-                  :name         "sum_double-price",
-                  :display-name "Sum of double-price",
-                  :lib/source   :source/aggregations}
-                 {:lib/type     :metadata/field,
-                  :base-type    :type/Integer,
-                  :name         "count",
-                  :display-name "Count",
-                  :lib/source   :source/aggregations}
-                 {:settings     {:is_priceless true},
-                  :lib/type     :metadata/field,
-                  :base-type    :type/Integer,
-                  :name         "sum_PRICE",
-                  :display-name "Sum of Price",
-                  :lib/source   :source/aggregations}]
+        (is (=? [{:lib/type       :metadata/field,
+                  :effective-type :type/Integer,
+                  :name           "sum_double-price",
+                  :display-name   "Sum of double-price",
+                  :lib/source     :source/aggregations}
+                 {:lib/type       :metadata/field,
+                  :effective-type :type/Integer,
+                  :name           "count",
+                  :display-name   "Count",
+                  :lib/source     :source/aggregations}
+                 {:settings       {:is_priceless true},
+                  :lib/type       :metadata/field,
+                  :effective-type :type/Integer,
+                  :name           "sum_PRICE",
+                  :display-name   "Sum of Price",
+                  :lib/source     :source/aggregations}]
                 (lib/aggregations-metadata agg-query)))))))
 
 (deftest ^:parallel selected-aggregation-operator-test
@@ -574,14 +575,27 @@
   (testing "Aggregation metadata should return the `:settings` for the field being aggregated, for some reason."
     (let [query (-> (lib/query-for-table-name meta/metadata-provider "VENUES")
                     (lib/aggregate (lib/sum (lib/field (meta/id :venues :price)))))]
-      (is (=? {:settings     {:is_priceless true}
-               :lib/type     :metadata/field
-               :base-type    :type/Integer
-               :name         "sum_PRICE"
-               :display-name "Sum of Price"
-               :lib/source   :source/aggregations}
+      (is (=? {:settings       {:is_priceless true}
+               :lib/type       :metadata/field
+               :effective-type :type/Integer
+               :name           "sum_PRICE"
+               :display-name   "Sum of Price"
+               :lib/source     :source/aggregations}
               (lib.metadata.calculation/metadata query (first (lib/aggregations-metadata query -1))))))))
 
+(deftest ^:parallel count-aggregation-type-test
+  (testing "Count aggregation should produce numeric columns"
+    (let [query (-> (lib/query-for-table-name meta/metadata-provider "VENUES")
+                    (lib/aggregate (lib/count)))
+          count-meta (first (lib/aggregations-metadata query -1))]
+      (is (=? {:lib/type       :metadata/field
+               :effective-type :type/Integer
+               :name           "count"
+               :display-name   "Count"
+               :lib/source     :source/aggregations}
+              count-meta))
+      (is (lib.types.isa/numeric? count-meta)))))
+
 (deftest ^:parallel var-test
   (let [query (-> (lib/query-for-table-name meta/metadata-provider "VENUES")
                   (lib/aggregate (lib/var (lib/field (meta/id :venues :price)))))]