Sort drill should remove ALL existing order bys (#38380)

src/metabase/lib/drill_thru/sort.cljc

@@ -24,7 +24,6 @@
    [metabase.lib.equality :as lib.equality]
    [metabase.lib.order-by :as lib.order-by]
    [metabase.lib.ref :as lib.ref]
-   [metabase.lib.remove-replace :as lib.remove-replace]
    [metabase.lib.schema :as lib.schema]
    [metabase.lib.schema.drill-thru :as lib.schema.drill-thru]
    [metabase.lib.schema.order-by :as lib.schema.order-by]
@@ -82,12 +81,10 @@
     stage-number                 :- :int
     {:keys [column], :as _drill} :- ::lib.schema.drill-thru/drill-thru.sort
     direction                    :- ::lib.schema.order-by/direction]
-   ;; if you have an existing order by, the drill thru returned by [[sort-drill]] would only be one that would suggest
-   ;; changing it to the opposite direction, so we can safely assume we want to change the direction and
-   ;; use [[lib.order-by/change-direction]] here.
-   (if-let [existing-clause (existing-order-by-clause query stage-number column)]
-     (lib.remove-replace/replace-clause query existing-clause (lib.order-by/order-by-clause column (keyword direction)))
-     (lib.order-by/order-by query stage-number column (keyword direction)))))
+   (-> query
+       ;; remove all existing order bys (see #37633), then add the new one.
+       (lib.order-by/remove-all-order-bys stage-number)
+       (lib.order-by/order-by stage-number column (keyword direction)))))
 
 (defmethod lib.drill-thru.common/drill-thru-info-method :drill-thru/sort
   [_query _stage-number {directions :sort-directions}]

src/metabase/lib/order_by.cljc

@@ -176,3 +176,12 @@
      (mbql.u.match/replace query
        [direction (_ :guard #(= (:lib/uuid %) lib-uuid)) _]
        (assoc &match 0 (opposite-direction direction))))))
+
+(mu/defn remove-all-order-bys :- ::lib.schema/query
+  "Remove all order bys from this stage of the query."
+  ([query]
+   (remove-all-order-bys query -1))
+
+  ([query        :- ::lib.schema/query
+    stage-number :- :int]
+   (lib.util/update-query-stage query stage-number dissoc :order-by)))

test/metabase/lib/drill_thru/sort_test.cljc

@@ -85,7 +85,7 @@
                   (lib/drill-thru query -1 drill :desc))))))))
 
 (deftest ^:parallel remove-existing-sort-test
-  (testing "Applying sort to already sorted column should REPLACE original sort (#34497)"
+  (testing "Applying sort to already sorted column should REPLACE original sort (#34497, #37633)"
     ;; technically this query doesn't make sense, how are you supposed to have a max aggregation and then also order
     ;; by a different column, but MBQL doesn't enforce that,
     (let [query   (-> (lib/query meta/metadata-provider (meta/table-metadata :orders))
@@ -107,8 +107,7 @@
               drill))
       (testing "We should REPLACE the original sort, as opposed to removing it and appending a new one"
         (is (=? {:stages
-                 [{:order-by [[:desc {} [:field {} (meta/id :orders :user-id)]]
-                              [:asc {} [:field {} (meta/id :orders :id)]]]}]}
+                 [{:order-by [[:desc {} [:field {} (meta/id :orders :user-id)]]]}]}
                 (lib/drill-thru query -1 drill :desc)))))))
 
 (deftest ^:parallel returns-sort-test-1

test/metabase/lib/order_by_test.cljc

@@ -7,6 +7,7 @@
    [metabase.lib.core :as lib]
    [metabase.lib.metadata :as lib.metadata]
    [metabase.lib.metadata.calculation :as lib.metadata.calculation]
+   [metabase.lib.order-by :as lib.order-by]
    [metabase.lib.query :as lib.query]
    [metabase.lib.test-metadata :as meta]
    [metabase.lib.test-util :as lib.tu]
@@ -782,3 +783,12 @@
       (is (= opposite (first new-order-by)))
       (is (= (assoc-in query [:stages 0 :order-by 0 0] opposite)
              new-query)))))
+
+(deftest ^:parallel remove-all-order-bys-test
+  (let [query (-> (lib/query meta/metadata-provider (meta/table-metadata :venues))
+                  (lib/order-by (meta/field-metadata :venues :id)))]
+    (is (=? {:stages [{:order-by [[:asc {} [:field {} (meta/id :venues :id)]]]}]}
+            query))
+    (let [query' (lib.order-by/remove-all-order-bys query)]
+      (is (=? {:stages [{:order-by (symbol "nil #_\"key is not present.\"")}]}
+              query')))))