[MLv2] Disambiguate columns: `:field` refs shouldn't match expressions (#35872)

Expressions which just wrap a real Field get `:id`, `:table_id` etc. so
they match a `[:field {} 7]` ref. If that real field exists (table
default or joined), prefer that over the expressions.

src/metabase/lib/equality.cljc

@@ -158,15 +158,36 @@
         (not-empty (filter #(clojure.core/= (:id %) ref-id) columns)))
       []))
 
+(defn- ambiguous-match-error [a-ref columns]
+  (ex-info "Ambiguous match! Implement more logic in disambiguate-matches."
+           {:ref a-ref
+            :columns columns}))
+
+(mu/defn ^:private expression-column? [column]
+  (or (= (:lib/source column) :source/expressions)
+      (:lib/expression-name column)))
+
+(mu/defn ^:private disambiguate-matches-dislike-field-refs-to-expressions :- [:maybe ::lib.schema.metadata/column]
+  "If a custom column is a simple wrapper for a field, that column gets `:id`, `:table_id`, etc.
+  A custom column should get a ref like `[:expression {} \"expr name\"]`, not `[:field {} 17]`.
+  If we got a `:field` ref, prefer matches which are not `:lib/source :source/expressions`."
+  [a-ref   :- ::lib.schema.ref/ref
+   columns :- [:sequential ::lib.schema.metadata/column]]
+  (or (when (= (first a-ref) :field)
+        (when-let [non-exprs (not-empty (remove expression-column? columns))]
+          (when-not (next non-exprs)
+            (first non-exprs))))
+      ;; In all other cases, this is an ambiguous match.
+      (throw (ambiguous-match-error a-ref columns))))
+
 (mu/defn ^:private disambiguate-matches-prefer-explicit :- [:maybe ::lib.schema.metadata/column]
+  "Prefers table-default or explicitly joined columns over implicitly joinable ones."
   [a-ref   :- ::lib.schema.ref/ref
    columns :- [:sequential ::lib.schema.metadata/column]]
   (if-let [no-implicit (not-empty (remove :fk-field-id columns))]
     (if-not (next no-implicit)
       (first no-implicit)
-      (throw (ex-info "Ambiguous match! Implement more logic in disambiguate-matches."
-                      {:ref a-ref
-                       :columns columns})))
+      (disambiguate-matches-dislike-field-refs-to-expressions a-ref no-implicit))
     nil))
 
 (mu/defn ^:private disambiguate-matches-no-alias :- [:maybe ::lib.schema.metadata/column]

test/metabase/lib/equality_test.cljc

@@ -545,3 +545,21 @@
       (is (= (map :name returned)
              (for [col returned]
                (:name (lib.equality/find-matching-column query -1 (lib/ref col) returned))))))))
+
+(deftest ^:parallel field-refs-to-custom-expressions-test
+  (testing "custom columns that wrap a Field have `:id` - prefer matching `[:field {} 7]` to the regular field (#35839)"
+    (let [query      (-> (lib/query meta/metadata-provider (meta/table-metadata :orders))
+                         (lib/expression "CA" (meta/field-metadata :orders :created-at)))
+          columns    (lib/visible-columns query)
+          created-at (m/find-first #(= (:name %) "CREATED_AT") columns)
+          ca-expr    (m/find-first #(= (:name %) "CA") columns)]
+      (testing "different columns"
+        (is (not= created-at ca-expr))
+        (testing "but both have the ID"
+          (is (= (:id created-at) (:id ca-expr)))))
+
+      (testing "both refs should match correctly"
+        (is (= created-at
+               (lib.equality/find-matching-column (lib/ref created-at) columns)))
+        (is (= ca-expr
+               (lib.equality/find-matching-column (lib/ref ca-expr)    columns)))))))