diff --git a/src/metabase/query_processor/pivot.clj b/src/metabase/query_processor/pivot.clj index 7ef4353638c5cf30b9481c2953038f45fa1c6e27..f47e4b933313aa94bae7ab9c3da28743b8d258d0 100644 --- a/src/metabase/query_processor/pivot.clj +++ b/src/metabase/query_processor/pivot.clj @@ -361,12 +361,16 @@ query (lib/query metadata-provider query) returned-columns (lib/returned-columns query) {:source/keys [aggregations breakouts]} (group-by :lib/source returned-columns) + column-alias->index (into {} + (map-indexed (fn [i column] [(:lib/desired-column-alias column) i])) + (concat breakouts aggregations)) column-name->index (into {} (map-indexed (fn [i column] [(:name column) i])) (concat breakouts aggregations)) process-columns (fn process-columns [column-names] (when (seq column-names) - (into [] (keep column-name->index) column-names))) + (into [] (keep (fn [n] (or (column-alias->index n) + (column-name->index n)))) column-names))) pivot-opts {:pivot-rows (process-columns rows) :pivot-cols (process-columns columns) :pivot-measures (process-columns values)}] diff --git a/src/metabase/query_processor/pivot/postprocess.clj b/src/metabase/query_processor/pivot/postprocess.clj index c52d0b38e859c0b7fd9312062ac4dacf9f3f6c77..95bc682977b220b5e7881554ac2f971a9ad82391 100644 --- a/src/metabase/query_processor/pivot/postprocess.clj +++ b/src/metabase/query_processor/pivot/postprocess.clj @@ -129,15 +129,19 @@ :min (fn [prev v] (if (number? v) - (-> (merge {:min 0} prev) - (update :min #(min % v))) + (update prev :min + (fn [x] (if x + (min x v) + v))) v)) :max (fn [prev v] (if (number? v) - (-> (merge {:min 0} prev) - (update :min #(max % v))) + (update prev :max + (fn [x] (if x + (max x v) + v))) v)) ;; else @@ -442,10 +446,11 @@ ;; filtering out any rows that begin with "Totals ..." (mapv (fn [row] - (let [[row-part vals-part] (split-at (count pivot-rows) row)] + (let [[row-part vals-part] (split-at (count pivot-rows) row) + first-entry (first row-part)] (if (or (not (seq row-part)) - (str/starts-with? (first row-part) "Totals")) + (and (string? first-entry) (str/starts-with? first-entry "Totals"))) row (into (mapv fmt row-formatters row-part) vals-part))))))) sections-rows)))) diff --git a/src/metabase/query_processor/streaming/csv.clj b/src/metabase/query_processor/streaming/csv.clj index 8efde3e9b1dc0c9a1164166ca9d299645167e26a..983a63aa6cdeeb1590242b13834fe804f8174265 100644 --- a/src/metabase/query_processor/streaming/csv.clj +++ b/src/metabase/query_processor/streaming/csv.clj @@ -116,7 +116,7 @@ (mapv #(formatter/create-formatter results_timezone % viz-settings format-rows?) ordered-cols)) ;; write the column names for non-pivot tables - (when (not opts) + (when (or (not opts) (not (public-settings/enable-pivoted-exports))) (let [header (m/remove-nth (or pivot-grouping-key (inc (count col-names))) col-names)] (write-csv writer [header]) (.flush writer))))) diff --git a/test/metabase/api/downloads_exports_test.clj b/test/metabase/api/downloads_exports_test.clj index b2c600772ac72e60cf870293b495b4f62a6940a1..fa5597574a59e7074a52b40be81a77a11a79a04c 100644 --- a/test/metabase/api/downloads_exports_test.clj +++ b/test/metabase/api/downloads_exports_test.clj @@ -313,7 +313,8 @@ ((fn [m] (update-vals m #(into #{} (mapv first %))))) (apply concat))))) (testing "only when `public-settings/enable-pivoted-exports` is true (true by default)." - (is (= [[["Doohickey" "2016" "$632.14"] + (is (= [[["Category" "Created At: Year" "Sum of Price"] + ["Doohickey" "2016" "$632.14"] ["Doohickey" "2017" "$854.19"] ["Doohickey" "2018" "$496.43"] ["Doohickey" "2019" "$203.13"] @@ -1433,3 +1434,190 @@ (if expected (is (= [nil nil] [unique-to-a unique-to-b])) (is (or (some? unique-to-a) (some? unique-to-b)))))))))))) + +(deftest ^:parallel pivot-exports-handle-nil-in-breakout-column + (testing "Pivot Exports will still work if the breakout column contains `nil` values." + (let [q "SELECT A, + CASE + WHEN A = 2 THEN NULL + ELSE A + END AS MEASURE + FROM ( SELECT 1 AS A UNION ALL SELECT 2 UNION ALL SELECT 3 )"] + (mt/dataset test-data + (mt/with-temp [:model/Card {pivot-data-card-id :id} + {:dataset_query {:database (mt/id) + :type :native + :native + {:template-tags {} + :query q}} + :result_metadata + (into [] (for [[_ field-name {:keys [base-type]}] pivot-fields] + {:name field-name + :display_name field-name + :field_ref [:field field-name {:base-type base-type}] + :base_type base-type}))} + :model/Card pivot-card + {:display :pivot + :visualization_settings {:pivot_table.column_split + {:rows ["MEASURE"] + :columns [] + :values ["count" "sum"]}} + :dataset_query {:database (mt/id) + :type :query + :query + {:breakout [[:field "MEASURE" {:base-type :type/Integer}]], + :aggregation + [[:count] + [:sum [:field "A" {:base-type :type/Integer}]]] + :source-table (format "card__%s" pivot-data-card-id)}}}] + (let [result (card-download pivot-card {:export-format :csv :pivot true})] + (is (= [["MEASURE" "Count" "Sum of A"] + ["" "1" "2"] + ["1" "1" "1"] + ["3" "1" "3"] + ["Grand totals" "3" "6"]] + result)))))))) + +(deftest ^:parallel pivot-exports-handle-aggregations-with-the-same-base-name + (testing "Pivot Exports with multiple of the same kind of aggregation will include all of the data." + (let [q "SELECT A, B, MEASURE + FROM ( + SELECT 1 as A, 1 as B, 1 as MEASURE UNION ALL + SELECT 2, 2, 2 UNION ALL + SELECT 3, 3, 3 UNION ALL + SELECT 4, 4, 4 UNION ALL + SELECT 5, 5, 5 + )"] + (mt/dataset test-data + (mt/with-temp [:model/Card {pivot-data-card-id :id} + {:dataset_query {:database (mt/id) + :type :native + :native + {:template-tags {} + :query q}} + :result_metadata + (into [] (for [[_ field-name {:keys [base-type]}] pivot-fields] + {:name field-name + :display_name field-name + :field_ref [:field field-name {:base-type base-type}] + :base_type base-type}))} + :model/Card pivot-card + {:display :pivot + :visualization_settings {:pivot_table.column_split + {:rows ["MEASURE"] + :columns [] + :values ["count" "sum" "sum_2"]}} + :dataset_query {:database (mt/id) + :type :query + :query + {:breakout [[:field "MEASURE" {:base-type :type/Integer}]], + :aggregation + [[:count] + [:sum [:field "A" {:base-type :type/Integer}]] + [:sum [:field "B" {:base-type :type/Integer}]]] + :source-table (format "card__%s" pivot-data-card-id)}}} + :model/Card reordered-card + {:display :pivot + :visualization_settings {:pivot_table.column_split + {:rows ["MEASURE"] + :columns [] + :values ["sum_2" "count" "sum"]}} + :dataset_query {:database (mt/id) + :type :query + :query + {:breakout [[:field "MEASURE" {:base-type :type/Integer}]], + :aggregation + [[:count] + [:sum [:field "A" {:base-type :type/Integer}]] + [:sum [:field "B" {:base-type :type/Integer}]]] + :source-table (format "card__%s" pivot-data-card-id)}}}] + (let [result (card-download pivot-card {:export-format :csv :pivot true}) + reordered-result (card-download reordered-card {:export-format :csv :pivot true})] + (testing "Both Sums are correctly included." + (is (= [["MEASURE" "Count" "Sum of A" "Sum of B"] + ["1" "1" "1" "1"] + ["2" "1" "2" "2"] + ["3" "1" "3" "3"] + ["4" "1" "4" "4"] + ["5" "1" "5" "5"] + ["Grand totals" "5" "15" "15"]] + result))) + (testing "and different order still works." + (is (= [["MEASURE" "Sum of B" "Count" "Sum of A"] + ["1" "1" "1" "1"] + ["2" "2" "1" "2"] + ["3" "3" "1" "3"] + ["4" "4" "1" "4"] + ["5" "5" "1" "5"] + ["Grand totals" "15" "5" "15"]] + reordered-result))))))))) + +(deftest ^:parallel pivot-exports-aggregations-work + (testing "Pivot Exports have correct aggregations." + (let [q "SELECT A, B + FROM ( + SELECT 1 as A, 1 as B UNION ALL + SELECT 1, 2 UNION ALL + SELECT 1, 3 UNION ALL + SELECT 1, 4 UNION ALL + SELECT 1, 5 UNION ALL + SELECT 2, 10 UNION ALL + SELECT 2, 20 UNION ALL + SELECT 2, 30 UNION ALL + SELECT 2, 40 UNION ALL + SELECT 2, 50 UNION ALL + SELECT 3, -1 UNION ALL + SELECT 3, -2 UNION ALL + SELECT 3, -3 UNION ALL + SELECT 3, -4 UNION ALL + SELECT 3, -5 UNION ALL + SELECT 4, 15 UNION ALL + SELECT 4, 25 UNION ALL + SELECT 4, 35 UNION ALL + SELECT 4, 45 UNION ALL + SELECT 4, 55 UNION ALL + SELECT 5, 11 UNION ALL + SELECT 5, 22 UNION ALL + SELECT 5, 33 UNION ALL + SELECT 5, 44 UNION ALL + SELECT 5, 55 + )"] + (mt/dataset test-data + (mt/with-temp [:model/Card {pivot-data-card-id :id} + {:dataset_query {:database (mt/id) + :type :native + :native + {:template-tags {} + :query q}} + :result_metadata + (into [] (for [[_ field-name {:keys [base-type]}] pivot-fields] + {:name field-name + :display_name field-name + :field_ref [:field field-name {:base-type base-type}] + :base_type base-type}))} + :model/Card pivot-card + {:display :pivot + :visualization_settings {:pivot_table.column_split + {:rows ["A"] + :columns [] + :values ["count" "sum" "avg" "min" "max"]}} + :dataset_query {:database (mt/id) + :type :query + :query + {:breakout [[:field "A" {:base-type :type/Integer}]], + :aggregation + [[:count] + [:sum [:field "B" {:base-type :type/Integer}]] + [:avg [:field "B" {:base-type :type/Integer}]] + [:min [:field "B" {:base-type :type/Integer}]] + [:max [:field "B" {:base-type :type/Integer}]]] + :source-table (format "card__%s" pivot-data-card-id)}}}] + (let [result (card-download pivot-card {:export-format :csv :pivot true})] + (is (= [["A" "Count" "Sum of B" "Average of B" "Min of B" "Max of B"] + ["1" "5" "15" "3.0" "1" "5"] + ["2" "5" "150" "30.0" "10" "50"] + ["3" "5" "-15" "-3.0" "-5" "-1"] + ["4" "5" "175" "35.0" "15" "55"] + ["5" "5" "165" "33.0" "11" "55"] + ["Grand totals" "25" "490" "19.6" "-5" "55"]] + result))))))))