[MLv2] Handle removing multiple dependent joins (#35109)

src/metabase/lib/remove_replace.cljc

@@ -60,7 +60,8 @@
       stage-number
       lib.util/remove-clause
       [:order-by]
-      (get-in (lib.util/query-stage query stage-number) [:order-by order-by-idx]))
+      (get-in (lib.util/query-stage query stage-number) [:order-by order-by-idx])
+      stage-number)
     query))
 
 (defn- remove-replace-location
@@ -116,7 +117,7 @@
                    (stage-paths query stage-number))]
     (reduce
      (fn [query [location target-clause]]
-       (remove-replace-location query stage-number unmodified-query-for-stage location target-clause lib.util/remove-clause))
+       (remove-replace-location query stage-number unmodified-query-for-stage location target-clause #(lib.util/remove-clause %1 %2 %3 stage-number)))
      query
      to-remove)))
 
@@ -150,7 +151,7 @@
                                (lib.common/->op-arg replacement))
           remove-replace-fn (if replace?
                               #(lib.util/replace-clause %1 %2 %3 replacement-clause)
-                              lib.util/remove-clause)
+                              #(lib.util/remove-clause %1 %2 %3 stage-number))
           changing-breakout? (= [:breakout] location)
           sync-breakout-ordering? (and replace?
                                        changing-breakout?
@@ -343,10 +344,10 @@
                                                   (not-empty (filterv #(not (dependent-join? % join-alias))
                                                                       joins))))
      (catch #?(:clj Exception :cljs :default) e
-       (let [{:keys [error stage join]} (ex-data e)]
+       (let [{:keys [error join] error-stage-number :stage-number} (ex-data e)]
          (if (= error ::lib.util/cannot-remove-final-join-condition)
            (-> query
-               (remove-join (u/index-of #{stage} (:stages query)) join)
+               (remove-join error-stage-number join)
                (remove-join stage-number join-spec))
            (throw e)))))))
 

src/metabase/lib/util.cljc

@@ -115,7 +115,7 @@
    If `location` contains no clause with `target-clause` no removal happens.
    If the the location is empty, dissoc it from stage.
    For the [:fields] location if only expressions remain, dissoc from stage."
-  [stage location target-clause]
+  [stage location target-clause stage-number]
   {:pre [(clause? target-clause)]}
   (if-let [target (get-in stage location)]
     (let [target-uuid (lib.options/uuid target-clause)
@@ -133,6 +133,7 @@
                         {:error ::cannot-remove-final-join-condition
                          :conditions (get-in stage location)
                          :join (get-in stage (pop location))
+                         :stage-number stage-number
                          :stage stage}))
 
         (= [:joins :fields] [first-loc last-loc])

test/metabase/lib/remove_replace_test.cljc

@@ -938,3 +938,14 @@
                     first
                     :joins
                     (map :alias))))))))
+
+(deftest ^:parallel remove-first-in-long-series-of-join-test
+  (testing "Recursive join removal (#35049)"
+    (let [query (-> (lib/query meta/metadata-provider (meta/table-metadata :reviews))
+                    (lib/join (meta/table-metadata :products))
+                    (lib/join (lib/join-clause (meta/table-metadata :orders) [(lib/= (lib/with-join-alias (meta/field-metadata :products :id) "Products")
+                                                                                     (lib/with-join-alias (meta/field-metadata :orders :product-id) "Orders"))]))
+                    (lib/join (meta/table-metadata :people)))]
+      (is (=?
+            {:stages [(complement :joins)]}
+            (lib/remove-clause query -1 (first (lib/joins query))))))))