diff --git a/src/metabase/query_processor/pivot.clj b/src/metabase/query_processor/pivot.clj index c765ddc6423104ed0713a7ae563d29628eb93c35..4bb8732724acc4c768b811327b0080adbd11ab2d 100644 --- a/src/metabase/query_processor/pivot.clj +++ b/src/metabase/query_processor/pivot.clj @@ -2,6 +2,7 @@ "Pivot table actions for the query processor" (:require [clojure.core.async :as a] [clojure.tools.logging :as log] + [medley.core :as m] [metabase.mbql.normalize :as mbql.normalize] [metabase.query-processor :as qp] [metabase.query-processor.context :as qp.context] @@ -100,6 +101,13 @@ (log/tracef "Added pivot-grouping expression to query\n%s" (u/pprint-to-str 'yellow query)) query))) +(defn- remove-existing-order-bys + "Remove existing `:order-by` clauses from the query. Since we're adding our own breakouts (i.e. `GROUP BY` and `ORDER + BY` clauses) to do the pivot table stuff, existing `:order-by` clauses probably won't work -- `ORDER BY` isn't + allowed for fields that don't appear in `GROUP BY`." + [outer-query] + (m/dissoc-in outer-query [:query :order-by])) + (defn- generate-queries "Generate the additional queries to perform a generic pivot table" [{{all-breakouts :breakout} :query, :keys [pivot-rows pivot-cols query], :as outer-query}] @@ -109,7 +117,9 @@ :let [group-bitmask (group-bitmask (count all-breakouts) breakout-indices) new-breakouts (for [i breakout-indices] (nth all-breakouts i))]] - (add-grouping-field outer-query new-breakouts group-bitmask)) + (-> outer-query + remove-existing-order-bys + (add-grouping-field new-breakouts group-bitmask))) (catch Throwable e (throw (ex-info (tru "Error generating pivot queries") {:type qp.error-type/qp, :query query} diff --git a/test/metabase/query_processor/pivot_test.clj b/test/metabase/query_processor/pivot_test.clj index b4757d80dfbd83c519669735dbd29f79f41c71d4..ac5e601465de0298d30f51f28ff707b59cfe492b 100644 --- a/test/metabase/query_processor/pivot_test.clj +++ b/test/metabase/query_processor/pivot_test.clj @@ -303,3 +303,18 @@ result)) (is (= (mt/rows (qp.pivot/run-pivot-query query)) (mt/rows result)))))))))))) + +(deftest pivot-with-order-by-test + (testing "Pivot queries should work if there is an `:order-by` clause (#17198)" + (mt/dataset sample-dataset + (let [query (mt/mbql-query products + {:breakout [$category] + :aggregation [[:count]] + :order-by [[:asc $category]]})] + (is (= [["Doohickey" 0 42] + ["Gadget" 0 53] + ["Gizmo" 0 51] + ["Widget" 0 54] + [nil 1 200]] + (mt/rows + (qp.pivot/run-pivot-query query))))))))