diff --git a/src/metabase/lib/aggregation.cljc b/src/metabase/lib/aggregation.cljc index 45f77a06fc17ec08aa805dc15e1929d9c5f053ba..c9f1977c4432977ff69097c9177bfd495cc81a3b 100644 --- a/src/metabase/lib/aggregation.cljc +++ b/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 diff --git a/src/metabase/lib/stage.cljc b/src/metabase/lib/stage.cljc index bc436c49f7237889aad72b479186315d3774d430..c53fab0a0374f48824cbd707bdacf47feef4e151 100644 --- a/src/metabase/lib/stage.cljc +++ b/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. ;;; diff --git a/src/metabase/util.cljc b/src/metabase/util.cljc index 8e012a9e47da2bfea8c5ad7ed1d621fa6b8aa867..97f269a807e2e15d6789775ffc8a336f18f5bd59 100644 --- a/src/metabase/util.cljc +++ b/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))) diff --git a/test/metabase/lib/aggregation_test.cljc b/test/metabase/lib/aggregation_test.cljc index d6d3b1a4068ae6c50c83d3dd266f4c82d3cfb6ee..85f75de8db7403c9a74553f0d60d21fa23f94df9 100644 --- a/test/metabase/lib/aggregation_test.cljc +++ b/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)))))]