Squash and rebase onto `master` so this is not blocked waiting for upstream PR reviews. (#33016)

src/metabase/lib/field.cljc

@@ -78,31 +78,38 @@
                             (pr-str (mapv :lib/desired-column-alias column-metadatas))))
         nil)))
 
+(def ^:private ^:dynamic *recursive-column-resolution-by-name*
+  "Whether we're in a recursive call to [[resolve-column-name]] or not. Prevent infinite recursion (#32063)"
+  false)
+
 (mu/defn ^:private resolve-column-name :- [:maybe lib.metadata/ColumnMetadata]
   "String column name: get metadata from the previous stage, if it exists, otherwise if this is the first stage and we
   have a native query or a Saved Question source query or whatever get it from our results metadata."
   [query        :- ::lib.schema/query
    stage-number :- :int
    column-name  :- ::lib.schema.common/non-blank-string]
-  (let [previous-stage-number (lib.util/previous-stage-number query stage-number)
-        stage                 (if previous-stage-number
-                                (lib.util/query-stage query previous-stage-number)
-                                (lib.util/query-stage query stage-number))
-        ;; TODO -- it seems a little icky that the existence of `:metabase.lib.stage/cached-metadata` is leaking here,
-        ;; we should look in to fixing this if we can.
-        stage-columns         (or (:metabase.lib.stage/cached-metadata stage)
-                                  (get-in stage [:lib/stage-metadata :columns])
-                                  (when (:source-card stage)
-                                    (lib.metadata.calculation/visible-columns query stage-number stage))
-                                  (log/warn (i18n/tru "Cannot resolve column {0}: stage has no metadata" (pr-str column-name))))]
-    (when-let [column (and (seq stage-columns)
-                           (resolve-column-name-in-metadata column-name stage-columns))]
-      (cond-> column
-        previous-stage-number (-> (dissoc :id :table-id
-                                          ::binning ::temporal-unit)
-                                  (lib.join/with-join-alias nil)
-                                  (assoc :name (or (:lib/desired-column-alias column) (:name column)))
-                                  (assoc :lib/source :source/previous-stage))))))
+  (when-not *recursive-column-resolution-by-name*
+    (binding [*recursive-column-resolution-by-name* true]
+      (let [previous-stage-number (lib.util/previous-stage-number query stage-number)
+            stage                 (if previous-stage-number
+                                    (lib.util/query-stage query previous-stage-number)
+                                    (lib.util/query-stage query stage-number))
+            ;; TODO -- it seems a little icky that the existence of `:metabase.lib.stage/cached-metadata` is leaking
+            ;; here, we should look in to fixing this if we can.
+            stage-columns         (or (:metabase.lib.stage/cached-metadata stage)
+                                      (get-in stage [:lib/stage-metadata :columns])
+                                      (when (:source-card stage)
+                                        (lib.metadata.calculation/visible-columns query stage-number stage))
+                                      (log/warn (i18n/tru "Cannot resolve column {0}: stage has no metadata"
+                                                          (pr-str column-name))))]
+        (when-let [column (and (seq stage-columns)
+                               (resolve-column-name-in-metadata column-name stage-columns))]
+          (cond-> column
+            previous-stage-number (-> (dissoc :id :table-id
+                                              ::binning ::temporal-unit)
+                                      (lib.join/with-join-alias nil)
+                                      (assoc :name (or (:lib/desired-column-alias column) (:name column)))
+                                      (assoc :lib/source :source/previous-stage))))))))
 
 (mu/defn ^:private resolve-field-metadata :- lib.metadata/ColumnMetadata
   "Resolve metadata for a `:field` ref. This is part of the implementation

src/metabase/lib/query.cljc

@@ -10,6 +10,7 @@
    [metabase.lib.schema :as lib.schema]
    [metabase.lib.schema.id :as lib.schema.id]
    [metabase.lib.util :as lib.util]
+   [metabase.util :as u]
    [metabase.util.malli :as mu]))
 
 (defmethod lib.normalize/normalize :mbql/query
@@ -63,28 +64,34 @@
   (let [query (lib.convert/->pMBQL query)]
     (query-with-stages metadata-providerable (:stages query))))
 
-(defmulti ^:private ->query
+(defmulti ^:private query-method
   "Implementation for [[query]]."
   {:arglists '([metadata-providerable x])}
   (fn [_metadata-providerable x]
     (lib.dispatch/dispatch-value x))
   :hierarchy lib.hierarchy/hierarchy)
 
-(defmethod ->query :dispatch-type/map
+(defmethod query-method :dispatch-type/map
   [metadata-providerable query]
   (query-from-existing metadata-providerable query))
 
 ;;; this should already be a query in the shape we want, but let's make sure it has the database metadata that was
 ;;; passed in
-(defmethod ->query :mbql/query
+(defmethod query-method :mbql/query
   [metadata-providerable query]
   (assoc query :lib/metadata (lib.metadata/->metadata-provider metadata-providerable)))
 
-(defmethod ->query :metadata/table
+(defmethod query-method :metadata/table
   [metadata-providerable table-metadata]
   (query-with-stages metadata-providerable
                      [{:lib/type     :mbql.stage/mbql
-                       :source-table (:id table-metadata)}]))
+                       :source-table (u/the-id table-metadata)}]))
+
+(defmethod query-method :metadata/card
+  [metadata-providerable card-metadata]
+  (query-with-stages metadata-providerable
+                     [{:lib/type     :mbql.stage/mbql
+                       :source-card (u/the-id card-metadata)}]))
 
 (mu/defn query :- ::lib.schema/query
   "Create a new MBQL query from anything that could conceptually be an MBQL query, like a Database or Table or an
@@ -92,7 +99,7 @@
   it in separately -- metadata is needed for most query manipulation operations."
   [metadata-providerable :- lib.metadata/MetadataProviderable
    x]
-  (->query metadata-providerable x))
+  (query-method metadata-providerable x))
 
 (mu/defn query-from-legacy-inner-query :- ::lib.schema/query
   "Create a pMBQL query from a legacy inner query."

src/metabase/lib/schema.cljc

@@ -213,7 +213,7 @@
     (loop [visible-join-alias? (constantly false), i 0, [stage & more] stages]
       (let [visible-join-alias? (some-fn visible-join-alias? (visible-join-alias?-fn stage))]
         (or
-         (mbql.match/match-one (dissoc stage :joins :stage/metadata)
+         (mbql.match/match-one (dissoc stage :joins :stage/metadata) ; TODO isn't this supposed to be `:lib/stage-metadata`?
            [:field ({:join-alias (join-alias :guard (complement visible-join-alias?))} :guard :join-alias) _id-or-name]
            (str "Invalid :field reference in stage " i ": no join named " (pr-str join-alias)))
          (when (seq more)

test/metabase/lib/card_test.cljc

@@ -4,6 +4,7 @@
    [clojure.test :refer [deftest is testing]]
    [metabase.lib.core :as lib]
    [metabase.lib.metadata.calculation :as lib.metadata.calculation]
+   [metabase.lib.metadata.composed-provider :as lib.metadata.composed-provider]
    [metabase.lib.test-metadata :as meta]
    [metabase.lib.test-util :as lib.tu]
    [metabase.util :as u]
@@ -79,3 +80,16 @@
                {:lib/type :metadata/column
                 :name     "count"}]
               (lib.metadata.calculation/returned-columns query))))))
+
+(deftest ^:parallel visible-columns-use-result--metadata-test
+  (testing "visible-columns should use the Card's `:result-metadata` (regardless of what's actually in the Card)"
+    (let [venues-query (lib/query
+                        (lib.metadata.composed-provider/composed-metadata-provider
+                         (lib.tu/mock-metadata-provider
+                          {:cards [(assoc (:orders lib.tu/mock-cards) :dataset-query lib.tu/venues-query)]})
+                         meta/metadata-provider)
+                        (:orders lib.tu/mock-cards))]
+      (is (= ["ID" "SUBTOTAL" "TOTAL" "TAX" "DISCOUNT" "QUANTITY" "CREATED_AT" "PRODUCT_ID" "USER_ID"]
+             (mapv :name (get-in lib.tu/mock-cards [:orders :result-metadata]))))
+      (is (= ["ID" "SUBTOTAL" "TOTAL" "TAX" "DISCOUNT" "QUANTITY" "CREATED_AT" "PRODUCT_ID" "USER_ID"]
+             (mapv :name (lib.metadata.calculation/visible-columns venues-query)))))))

test/metabase/lib/expression_test.cljc

@@ -288,3 +288,14 @@
     (is (= ["ID" "NAME" "a" "b"] (expressionable-expressions-for-position 2)))
     (is (= (lib.metadata.calculation/visible-columns query)
            (lib/expressionable-columns query nil)))))
+
+(deftest ^:parallel infix-display-name-with-expressions-test
+  (testing "#32063"
+    (let [query (lib/query lib.tu/metadata-provider-with-mock-cards (:orders lib.tu/mock-cards))
+          query (-> query
+                    (lib/expression "Unit price" (lib//
+                                                  (lib.tu/field-literal-ref query "SUBTOTAL")
+                                                  (lib.tu/field-literal-ref query "QUANTITY"))))]
+      (is (= ["ID" "Subtotal" "Total" "Tax" "Discount" "Quantity" "Created At" "Product ID" "User ID" "Unit price"]
+             (map (partial lib/display-name query)
+                  (lib.metadata.calculation/returned-columns query)))))))

test/metabase/lib/test_util.cljc

@@ -8,10 +8,13 @@
    [metabase.lib.convert :as lib.convert]
    [metabase.lib.core :as lib]
    [metabase.lib.metadata :as lib.metadata]
+   [metabase.lib.metadata.calculation :as lib.metadata.calculation]
    [metabase.lib.metadata.composed-provider :as lib.metadata.composed-provider]
    [metabase.lib.metadata.protocols :as metadata.protocols]
    [metabase.lib.query :as lib.query]
    [metabase.lib.schema :as lib.schema]
+   [metabase.lib.schema.common :as lib.schema.common]
+   [metabase.lib.schema.ref :as lib.schema.ref]
    [metabase.lib.test-metadata :as meta]
    [metabase.lib.util :as lib.util]
    [metabase.util.malli :as mu]
@@ -263,7 +266,22 @@
   (lib.metadata.composed-provider/composed-metadata-provider
     meta/metadata-provider
     (mock-metadata-provider
-     {:cards (vals mock-cards)})))
+      {:cards (vals mock-cards)})))
+
+(mu/defn field-literal-ref :- ::lib.schema.ref/field.literal
+  "Get a `:field` 'literal' ref (a `:field` ref that uses a string column name rather than an integer ID) for a column
+  with `column-name` returned by a `query`. This only makes sense for queries with multiple stages, or ones with a
+  source Card."
+  [query       :- ::lib.schema/query
+   column-name :- ::lib.schema.common/non-blank-string]
+  (let [cols     (lib.metadata.calculation/visible-columns query)
+        metadata (or (m/find-first #(= (:name %) column-name)
+                                   cols)
+                     (let [col-names (vec (sort (map :name cols)))]
+                       (throw (ex-info (str "No column named " (pr-str column-name) "; found: " (pr-str col-names))
+                                       {:column column-name
+                                        :found  col-names}))))]
+    (lib/ref metadata)))
 
 (mu/defn query-with-mock-card-as-source-card :- [:and
                                                  ::lib.schema/query