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

Enable type info computation for string id fields (#46488) (#46819)


* Enable type info computation for fields coming from model source query

* Add test

* Add handling for fields coming from joins

Co-authored-by: default avatarlbrdnk <lbrdnk@users.noreply.github.com>
parent a745bb7e
No related branches found
No related tags found
No related merge requests found
...@@ -21,6 +21,12 @@ ...@@ -21,6 +21,12 @@
;;; --------------------------------------------------- Type Info ---------------------------------------------------- ;;; --------------------------------------------------- Type Info ----------------------------------------------------
(def ^:private ^:dynamic *inner-query*
"To be bound in [[metabase.query-processor.middleware.wrap-value-literals/wrap-value-literals-in-mbql-query]].
Original motivation is to provide metadata required for computation of _type info_. See the
[[metabase.query-processor.middleware.wrap-value-literals/str-id-field->type-info]] docstring for details."
nil)
(defmulti ^:private type-info (defmulti ^:private type-info
"Get information about database, base, and semantic types for an object. This is passed to along to various "Get information about database, base, and semantic types for an object. This is passed to along to various
`->honeysql` method implementations so drivers have the information they need to handle raw values like Strings, `->honeysql` method implementations so drivers have the information they need to handle raw values like Strings,
...@@ -43,12 +49,27 @@ ...@@ -43,12 +49,27 @@
(when (types/temporal-field? field-info) (when (types/temporal-field? field-info)
{:unit :default})))) {:unit :default}))))
(defmethod type-info :field [[_ id-or-name opts]] (defn- str-id-field->type-info
"Return _type info_ for `_field` with string `field-name`, coming from the source query or joins."
[[_tag field-name {:keys [join-alias] :as _opts} :as _field] inner-query]
(when (string? field-name)
;; Use corresponding source-metadata from joins or `inner-query`.
(let [source-metadatas (if join-alias
(some #(when (= join-alias (:alias %))
(:source-metadata %))
(:joins inner-query))
(:source-metadata inner-query))]
(some #(when (= (:name %) field-name)
(select-keys % [:base_type :effective_type :database_type]))
source-metadatas))))
(defmethod type-info :field [[_ id-or-name opts :as field]]
(merge (merge
;; With Mlv2 queries, this could be combined with `:expression` below and use the column from the ;; With Mlv2 queries, this could be combined with `:expression` below and use the column from the
;; query rather than metadata/field ;; query rather than metadata/field
(when (integer? id-or-name) (if (integer? id-or-name)
(type-info (lib.metadata/field (qp.store/metadata-provider) id-or-name))) (type-info (lib.metadata/field (qp.store/metadata-provider) id-or-name))
(str-id-field->type-info field *inner-query*))
(when (:temporal-unit opts) (when (:temporal-unit opts)
{:unit (:temporal-unit opts)}) {:unit (:temporal-unit opts)})
(when (:base-type opts) (when (:base-type opts)
...@@ -296,7 +317,8 @@ ...@@ -296,7 +317,8 @@
[{:keys [source-query], :as inner-query} options] [{:keys [source-query], :as inner-query} options]
(let [inner-query (cond-> inner-query (let [inner-query (cond-> inner-query
source-query (update :source-query wrap-value-literals-in-mbql-query options))] source-query (update :source-query wrap-value-literals-in-mbql-query options))]
(wrap-value-literals-in-mbql inner-query))) (binding [*inner-query* inner-query]
(wrap-value-literals-in-mbql inner-query))))
(mu/defn wrap-value-literals :- mbql.s/Query (mu/defn wrap-value-literals :- mbql.s/Query
"Middleware that wraps ran value literals in `:value` (for integers, strings, etc.) or `:absolute-datetime` (for "Middleware that wraps ran value literals in `:value` (for integers, strings, etc.) or `:absolute-datetime` (for
......
...@@ -10,9 +10,11 @@ ...@@ -10,9 +10,11 @@
[metabase.lib.test-util :as lib.tu] [metabase.lib.test-util :as lib.tu]
[metabase.lib.test-util.macros :as lib.tu.macros] [metabase.lib.test-util.macros :as lib.tu.macros]
[metabase.query-processor.middleware.wrap-value-literals :as qp.wrap-value-literals] [metabase.query-processor.middleware.wrap-value-literals :as qp.wrap-value-literals]
[metabase.query-processor.preprocess :as qp.preprocess]
[metabase.query-processor.store :as qp.store] [metabase.query-processor.store :as qp.store]
[metabase.query-processor.timezone :as qp.timezone] [metabase.query-processor.timezone :as qp.timezone]
[metabase.test :as mt])) [metabase.test :as mt]
[toucan2.tools.with-temp :as toucan2.with-temp]))
(driver/register! ::tz-driver, :abstract? true) (driver/register! ::tz-driver, :abstract? true)
...@@ -301,3 +303,35 @@ ...@@ -301,3 +303,35 @@
lib.convert/->legacy-MBQL lib.convert/->legacy-MBQL
wrap-value-literals wrap-value-literals
(lib/query query)))))))) (lib/query query))))))))
(deftest ^:parallel model-source-type-info-test
(testing "type info is added to fields coming from model source query (#46059)"
;; Basically, this checks whether the [[metabase.query-processor.middleware.wrap-value-literals/type-info]] :field
;; adds options to values in expressions, where other arg is field clause with name instead of int id.
(toucan2.with-temp/with-temp [:model/Card {id :id} {:dataset_query (mt/mbql-query venues)
:type :model}]
(let [query (mt/mbql-query
venues
{:source-table (str "card__" id)
:filter [:= [:field "ID" {:base-type :type/Integer}] 1]})
preprocessed (qp.preprocess/preprocess query)]
;; [:query :filter 2 2 :database_type] points to wrapped value's options
(is (= "BIGINT" (get-in preprocessed [:query :filter 2 2 :database_type])))))))
(deftest ^:parallel model-join-type-info-test
(testing "type info is added to fields coming from join"
;; Basically, this checks whether the [[metabase.query-processor.middleware.wrap-value-literals/type-info]] :field
;; adds options to values in expressions, where other arg is field clause with name instead of int id.
(toucan2.with-temp/with-temp [:model/Card {id :id} {:dataset_query (mt/mbql-query venues)
:type :model}]
(let [query (mt/mbql-query
venues
{:filter [:= [:field "ID" {:base-type :type/Integer :join-alias "x"}] 1]
:joins [{:alias "x"
:condition [:=
$id
[:field "ID" {:base-type :type/Integer :join-alias "x"}]]
:source-table (str "card__" id)}]})
preprocessed (qp.preprocess/preprocess query)]
;; [:query :filter 2 2 :database_type] points to wrapped value's options
(is (= "BIGINT" (get-in preprocessed [:query :filter 2 2 :database_type])))))))
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