Skip to content
Snippets Groups Projects
Unverified Commit e83c367e authored by github-automation-metabase's avatar github-automation-metabase Committed by GitHub
Browse files

fix: uuid join conditions should not cast (#46614) (#46662)


Fixes: #46558

In #45575 we introduced casting uuid fields for drilldowns.

However we failed to include a check that the rhs was itself another
field of uuid type and so no casting should occur. This broke joins on
models when the database_type of the field may not be known.

Co-authored-by: default avatarCase Nelson <case@metabase.com>
parent c975b675
No related branches found
No related tags found
No related merge requests found
......@@ -1228,27 +1228,33 @@
expr
[:lower expr]))))
(defn- uuid-field?
[x]
(and (mbql.u/mbql-clause? x)
(isa? (or (:effective-type (get x 2))
(:base-type (get x 2)))
:type/UUID)))
(mu/defn ^:private maybe-cast-uuid-for-equality
"For := and :!=. Comparing UUID fields against non-uuid values requires casting."
[driver field arg]
(if (and (isa? (or (:effective-type (get field 2))
(:base-type (get field 2)))
:type/UUID)
;; If we could not convert the arg to a UUID then we have to cast the Field.
;; This will not hit indexes, but then we're passing an arg that can only be compared textually.
(not (uuid? (->honeysql driver arg)))
;; Check for inlined values
(not (= (:database-type (h2x/type-info (->honeysql driver arg))) "uuid")))
[::cast field "varchar"]
field))
(if (and (uuid-field? field)
;; If the arg is a uuid we are happy especially for joins (#46558)
(not (uuid-field? arg))
;; If we could not convert the arg to a UUID then we have to cast the Field.
;; This will not hit indexes, but then we're passing an arg that can only be compared textually.
(not (uuid? (->honeysql driver arg)))
;; Check for inlined values
(not (= (:database-type (h2x/type-info (->honeysql driver arg))) "uuid")))
[::cast field "varchar"]
field))
(mu/defn ^:private maybe-cast-uuid-for-text-compare
"For :contains, :starts-with, and :ends-with.
Comparing UUID fields against with these operations requires casting as the right side will have `%` for `LIKE` operations."
[field]
(if (isa? (or (:effective-type (get field 2))
(:base-type (get field 2)))
:type/UUID)
(if (uuid-field? field)
[::cast field "varchar"]
field))
......
......@@ -248,6 +248,7 @@
(= expected
(mt/rows (qp/process-query (assoc-in model-query [:query :filter] filt))))
[[uuid]] [:= (:field_ref col-metadata) [:value (str uuid) {:base_type :type/UUID}]]
[[uuid]] [:= (:field_ref col-metadata) (:field_ref col-metadata)]
[[uuid]] [:= (:field_ref col-metadata) (str uuid)]
[[uuid]] [:!= (:field_ref col-metadata) (str (random-uuid))]
[[uuid]] [:starts-with (:field_ref col-metadata) (str uuid)]
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment