Remove references to missing expressions when converting to pMBQL (#32685)

* Remove references to missing expressions when converting to pMBQL

Fixes #32625,

src/metabase/lib/convert.cljc

@@ -43,7 +43,7 @@
 (def ^:private stage-keys
   #{:aggregation :breakout :expressions :fields :filters :order-by :joins})
 
-(defn- clean-stage [almost-stage]
+(defn- clean-stage-schema-errors [almost-stage]
   (loop [almost-stage almost-stage
          removals []]
     (if-let [[error-type error-location] (->> (mc/explain ::lib.schema/stage.mbql almost-stage)
@@ -64,6 +64,17 @@
           (recur new-stage (conj removals [error-type error-location]))))
       almost-stage)))
 
+(defn- clean-stage-ref-errors [almost-stage]
+  (reduce (fn [almost-stage [loc _]]
+              (clean-location almost-stage ::lib.schema/invalid-ref loc))
+          almost-stage
+          (lib.schema/ref-errors-for-stage almost-stage)))
+
+(defn- clean-stage [almost-stage]
+  (-> almost-stage
+      clean-stage-schema-errors
+      clean-stage-ref-errors))
+
 (defn- clean [almost-query]
   (loop [almost-query almost-query
          stage-index 0]

src/metabase/lib/schema.cljc

@@ -60,17 +60,49 @@
 (mr/def ::filters
   [:sequential {:min 1} [:ref ::expression/boolean]])
 
-(defn- expression-ref-error-for-stage [stage]
+(defn- matching-locations
+  [form pred]
+  (loop [stack [[[] form]], matches []]
+    (if-let [[loc form :as top] (peek stack)]
+      (let [stack (pop stack)
+            onto-stack #(into stack (map (fn [[k v]] [(conj loc k) v])) %)]
+        (cond
+          (pred form)        (recur stack                                  (conj matches top))
+          (map? form)        (recur (onto-stack form)                      matches)
+          (sequential? form) (recur (onto-stack (map-indexed vector form)) matches)
+          :else              (recur stack                                  matches)))
+      matches)))
+
+(defn- bad-ref-clause? [ref-type valid-ids x]
+  (and (vector? x)
+       (= ref-type (first x))
+       (not (contains? valid-ids (get x 2)))))
+
+(defn- expression-ref-errors-for-stage [stage]
   (let [expression-names (into #{} (map (comp :lib/expression-name second)) (:expressions stage))]
-    (mbql.match/match-one (dissoc stage :joins :lib/stage-metadata)
-      [:expression _opts (expression-name :guard (complement expression-names))]
-      (str "Invalid :expression reference: no expression named " (pr-str expression-name)))))
+    (matching-locations (dissoc stage :joins :lib/stage-metadata)
+                        #(bad-ref-clause? :expression expression-names %))))
 
-(defn- aggregation-ref-error-for-stage [stage]
+(defn- aggregation-ref-errors-for-stage [stage]
   (let [uuids (into #{} (map (comp :lib/uuid second)) (:aggregation stage))]
-    (mbql.match/match-one (dissoc stage :joins :lib/stage-metadata)
-      [:aggregation _opts (ag-uuid :guard (complement uuids))]
-      (str "Invalid :aggregation reference: no aggregation with uuid " ag-uuid))))
+    (matching-locations (dissoc stage :joins :lib/stage-metadata)
+                        #(bad-ref-clause? :aggregation uuids %))))
+
+(defn ref-errors-for-stage
+  "Return the locations and the clauses with dangling expression or aggregation references.
+  The return value is sequence of pairs (vectors) with the first element specifying the location
+  as a vector usable in [[get-in]] and the second element being the clause with dangling reference."
+  [stage]
+  (concat (expression-ref-errors-for-stage stage)
+          (aggregation-ref-errors-for-stage stage)))
+
+(defn- expression-ref-error-for-stage [stage]
+  (when-let [err-loc (first (expression-ref-errors-for-stage stage))]
+    (str "Invalid :expression reference: no expression named " (pr-str (get-in err-loc [1 2])))))
+
+(defn- aggregation-ref-error-for-stage [stage]
+  (when-let [err-loc (first (aggregation-ref-errors-for-stage stage))]
+    (str "Invalid :aggregation reference: no aggregation with uuid " (get-in err-loc [1 2]))))
 
 (def ^:private ^{:arglists '([stage])} ref-error-for-stage
   "Validate references in the context of a single `stage`, independent of any previous stages. If there is an error with

test/metabase/lib/convert_test.cljc

@@ -595,4 +595,16 @@
                                  :fields [[:xfield 2 nil]]}]
                         :source-table 4}}
                lib.convert/->pMBQL
-               (get-in [:stages 0 :joins 0 :fields]))))))
+               (get-in [:stages 0 :joins 0 :fields]))))
+    (testing "references to missing expressions are removed (#32625)"
+      (let [query {:database 2762
+                   :type     :query
+                   :query    {:aggregation [[:sum [:case [[[:< [:field 139657 nil] 2] [:field 139657 nil]]] {:default 0}]]]
+                              :expressions {"custom" [:+ 1 1]}
+                              :breakout    [[:expression "expr1" nil] [:expression "expr2" nil]]
+                              :order-by    [[:expression "expr2" nil]]
+                              :limit       4
+                              :source-table 33674}}
+            converted (lib.convert/->pMBQL query)]
+        (is (empty? (get-in converted [:stages 0 :breakout])))
+        (is (empty? (get-in converted [:stages 0 :group-by])))))))