Fix pivot queries that specify `:order-by` (#21839)

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}

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))))))))