Skip to content

Commit

Permalink
[QP] When querying a bare model with parameters, wrap an extra stage
Browse files Browse the repository at this point in the history
This matches how models are visualized, and how the parameters are
attached to them in the first place - an extra ad-hoc query is wrapped
around the model.

Because of that wrapping, a parameter will be targeting a model's field
by name rather than by ID, but that parameter cannot be applied directly
on the model's last stage.

Fixes #39940.
  • Loading branch information
bshepherdson committed Mar 11, 2024
1 parent 8c27927 commit 0c5e249
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 4 deletions.
8 changes: 5 additions & 3 deletions src/metabase/query_processor/middleware/parameters.clj
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,17 @@

(mu/defn ^:private move-top-level-params-to-inner-query
"Move any top-level parameters to the same level (i.e., 'inner query') as the query they affect."
[{:keys [parameters], query-type :type, :as outer-query} :- [:map [:type [:enum :query :native]]]]
[{:keys [info parameters], query-type :type, :as outer-query} :- [:map [:type [:enum :query :native]]]]
(cond-> (set/rename-keys outer-query {:parameters :user-parameters})
(seq parameters)
(assoc-in [query-type :parameters] parameters)))
(and (seq parameters)
(:metadata/model-metadata info)) (update query-type (fn [inner-query] {:source-query inner-query}))
(seq parameters) (assoc-in [query-type :parameters] parameters)))

(defn- expand-parameters
"Expand parameters in the `outer-query`, and if the query is using a native source query, expand params in that as
well."
[outer-query]
#_(clojure.pprint/pprint [`expand-parameters outer-query])
(-> outer-query move-top-level-params-to-inner-query expand-all))

(mu/defn ^:private substitute-parameters* :- :map
Expand Down
23 changes: 22 additions & 1 deletion test/metabase/query_processor/middleware/parameters_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,28 @@
:native {:query "WOW", :parameters ["My Param"]}
:user-parameters ["My Param"]}
(#'parameters/move-top-level-params-to-inner-query
{:type :native, :native {:query "WOW"}, :parameters ["My Param"]}))))
{:type :native, :native {:query "WOW"}, :parameters ["My Param"]})))
(testing "when top-level query is a model"
(testing "and there are parameters, wrap it up as a :source-query"
(is (= {:type :query
:query {:source-query {:source-table 5}
:parameters ["My Param"]}
:info {:metadata/model-metadata []}
:user-parameters ["My Param"]}
(#'parameters/move-top-level-params-to-inner-query
{:type :query
:query {:source-table 5}
:parameters ["My Param"]
:info {:metadata/model-metadata []}}))))
(testing "without parameters, leave the model at the top level"
(is (= {:type :query
:query {:source-table 5
:parameters ["My Param"]}
:user-parameters ["My Param"]}
(#'parameters/move-top-level-params-to-inner-query
{:type :query
:query {:source-table 5}
:parameters ["My Param"]}))))))

(defn- substitute-params [query]
(letfn [(thunk []
Expand Down

0 comments on commit 0c5e249

Please sign in to comment.