Keep original join-aliases in expected columns (#30747)

* Keep original join-aliases in expected columns

Fixes #30648.

The root cause of the bug was that the result_metadata generated for MBQL
queries returned escaped join-aliases, which in BigQueries case means that
spaces are converted underscores (_). These wrong join aliases could not be
resolved in later stages and resulted in incorrect references.

Normally, the escaped aliases are replaced by original ones in the post
processing step, but the post processing is not executed when calculating
expected columns.

This PR makes sure the original aliases are restored in the expected columns.

* Keep source_alias and display_name intact too

src/metabase/query_processor.clj

@@ -307,6 +307,10 @@
               (preprocess* query)))]
     (qp query nil nil)))
 
+(defn- restore-join-aliases [preprocessed-query]
+  (let [replacement (-> preprocessed-query :info :alias/escaped->original)]
+    (escape-join-aliases/restore-aliases preprocessed-query replacement)))
+
 (defn query->expected-cols
   "Return the `:cols` you would normally see in MBQL query results by preprocessing the query and calling `annotate` on
   it. This only works for pure MBQL queries, since it does not actually run the queries. Native queries or MBQL
@@ -314,11 +318,11 @@
   [{query-type :type, :as query}]
   (when-not (= (mbql.u/normalize-token query-type) :query)
     (throw (ex-info (tru "Can only determine expected columns for MBQL queries.")
-             {:type qp.error-type/qp})))
+                    {:type qp.error-type/qp})))
   ;; TODO - we should throw an Exception if the query has a native source query or at least warn about it. Need to
   ;; check where this is used.
   (qp.store/with-store
-    (let [preprocessed (preprocess query)]
+    (let [preprocessed (-> query preprocess restore-join-aliases)]
       (driver/with-driver (driver.u/database->driver (:database preprocessed))
         (not-empty (vec (annotate/merged-column-info preprocessed nil)))))))
 

test/metabase/query_processor/util/add_alias_info_test.clj

@@ -570,6 +570,45 @@
                             add-alias-info
                             :query)))))))))
 
+(deftest query->expected-cols-test
+  (testing "field_refs in expected columns have the original join aliases (#30648)"
+    (mt/dataset sample-dataset
+      (binding [driver/*driver* ::custom-escape-spaces-to-underscores]
+        (let [query
+              (mt/mbql-query
+               products
+                {:joins
+                 [{:source-query
+                   {:source-table $$orders
+                    :joins
+                    [{:source-table $$people
+                      :alias "People"
+                      :condition [:= $orders.user_id &People.people.id]
+                      :fields [&People.people.address]
+                      :strategy :left-join}]
+                    :fields [$orders.id &People.people.address]}
+                   :alias "Question 54"
+                   :condition [:= $id [:field %orders.id {:join-alias "Question 54"}]]
+                   :fields [[:field %orders.id {:join-alias "Question 54"}]
+                            [:field %people.address {:join-alias "Question 54"}]]
+                   :strategy :left-join}]
+                 :fields
+                 [!default.created_at
+                  [:field %orders.id {:join-alias "Question 54"}]
+                  [:field %people.address {:join-alias "Question 54"}]]})]
+          (is (=? [{:name "CREATED_AT"
+                    :field_ref [:field (mt/id :products :created_at) {:temporal-unit :default}]
+                    :display_name "Created At"}
+                   {:name "ID"
+                    :field_ref [:field (mt/id :orders :id) {:join-alias "Question 54"}]
+                    :display_name "Question 54 → ID"
+                    :source_alias "Question 54"}
+                   {:name "ADDRESS"
+                    :field_ref [:field (mt/id :people :address) {:join-alias "Question 54"}]
+                    :display_name "Question 54 → Address"
+                    :source_alias "Question 54"}]
+                  (qp/query->expected-cols query))))))))
+
 (deftest use-source-unique-aliases-test
   (testing "Make sure uniquified aliases in the source query end up getting used for `::add/source-alias`"
     ;; keep track of the IDs so we don't accidentally fetch the wrong ones after we switch the name of `price`