diff --git a/src/metabase/query_processor/middleware/fetch_source_query.clj b/src/metabase/query_processor/middleware/fetch_source_query.clj index 6f611a59db324e190cea5313094ef8dc7cfb606b..6cd8d6df0889403d83a6b00bb1b2502c251c2b9f 100644 --- a/src/metabase/query_processor/middleware/fetch_source_query.clj +++ b/src/metabase/query_processor/middleware/fetch_source_query.clj @@ -292,7 +292,7 @@ remove-unneeded-database-ids extract-resolved-card-id)) -(s/defn ^:private resolve-card-id-source-tables* :- {:card-id (s/maybe su/IntGreaterThanZero) +(s/defn resolve-card-id-source-tables* :- {:card-id (s/maybe su/IntGreaterThanZero) :query FullyResolvedQuery} "Resolve `card__n`-style `:source-tables` in `query`." [{inner-query :query, :as outer-query} :- mbql.s/Query] diff --git a/src/metabase/query_processor/middleware/permissions.clj b/src/metabase/query_processor/middleware/permissions.clj index a442ed2bb45ff85a5a291d87b24a3a324e02d8b4..18da7350991e48ebd1f5b165f6eb2d57c7f73185 100644 --- a/src/metabase/query_processor/middleware/permissions.clj +++ b/src/metabase/query_processor/middleware/permissions.clj @@ -10,8 +10,8 @@ [metabase.models.query.permissions :as query-perms] [metabase.plugins.classloader :as classloader] [metabase.query-processor.error-type :as qp.error-type] - [metabase.query-processor.middleware.resolve-referenced - :as qp.resolve-referenced] + [metabase.query-processor.util.tag-referenced-cards + :as qp.u.tag-referenced-cards] [metabase.util :as u] [metabase.util.i18n :refer [tru]] [metabase.util.log :as log] @@ -86,7 +86,7 @@ (when-not (has-data-perms? required-perms) (throw (perms-exception required-perms)))) ;; check perms for any Cards referenced by this query (if it is a native query) - (doseq [{query :dataset_query} (qp.resolve-referenced/tags-referenced-cards outer-query)] + (doseq [{query :dataset_query} (qp.u.tag-referenced-cards/tags-referenced-cards outer-query)] (check-query-permissions* query))) (s/defn ^:private check-query-permissions* diff --git a/src/metabase/query_processor/middleware/resolve_referenced.clj b/src/metabase/query_processor/middleware/resolve_referenced.clj index 05867395d61090837b590d7c22dae0b1814825bf..8d3cda3c87208432072b71462497617888392774 100644 --- a/src/metabase/query_processor/middleware/resolve_referenced.clj +++ b/src/metabase/query_processor/middleware/resolve_referenced.clj @@ -1,10 +1,14 @@ (ns metabase.query-processor.middleware.resolve-referenced (:require [metabase.models.card :refer [Card]] + [metabase.query-processor.middleware.fetch-source-query + :as fetch-source-query] [metabase.query-processor.middleware.resolve-fields :as qp.resolve-fields] [metabase.query-processor.middleware.resolve-source-table :as qp.resolve-source-table] + [metabase.query-processor.util.tag-referenced-cards + :as qp.u.tag-referenced-cards] [metabase.util.i18n :refer [tru]] [schema.core :as s] [toucan.db :as db] @@ -12,25 +16,6 @@ (:import (clojure.lang ExceptionInfo))) -(defn- query->template-tags - [query] - (vals (get-in query [:native :template-tags]))) - -(defn- query->tag-card-ids - [query] - (keep :card-id (query->template-tags query))) - -(defn tags-referenced-cards - "Returns Card instances referenced by the given native `query`." - [query] - (mapv - (fn [card-id] - (if-let [card (db/select-one Card :id card-id)] - card - (throw (ex-info (tru "Referenced question #{0} could not be found" (str card-id)) - {:card-id card-id})))) - (query->tag-card-ids query))) - (defn- check-query-database-id= [query database-id] (when-not (= (:database query) database-id) @@ -40,11 +25,12 @@ (s/defn ^:private resolve-referenced-card-resources* :- clojure.lang.IPersistentMap [query] - (doseq [referenced-card (tags-referenced-cards query) - :let [referenced-query (:dataset_query referenced-card)]] + (doseq [referenced-card (qp.u.tag-referenced-cards/tags-referenced-cards query) + :let [referenced-query (:dataset_query referenced-card) + resolved-query (fetch-source-query/resolve-card-id-source-tables* referenced-query)]] (check-query-database-id= referenced-query (:database query)) - (qp.resolve-source-table/resolve-source-tables referenced-query) - (qp.resolve-fields/resolve-fields referenced-query)) + (qp.resolve-source-table/resolve-source-tables resolved-query) + (qp.resolve-fields/resolve-fields resolved-query)) query) (defn- card-subquery-graph @@ -55,7 +41,7 @@ (card-subquery-graph (dep/depend g card-id sub-card-id) sub-card-id)) graph - (query->tag-card-ids card-query)))) + (qp.u.tag-referenced-cards/query->tag-card-ids card-query)))) (defn- circular-ref-error [from-card to-card] @@ -68,7 +54,7 @@ [query] (try ;; `card-subquery-graph` will throw if there are circular references - (reduce card-subquery-graph (dep/graph) (query->tag-card-ids query)) + (reduce card-subquery-graph (dep/graph) (qp.u.tag-referenced-cards/query->tag-card-ids query)) (catch ExceptionInfo e (let [{:keys [reason node dependency]} (ex-data e)] (if (= reason :weavejester.dependency/circular-dependency) diff --git a/src/metabase/query_processor/util/tag_referenced_cards.clj b/src/metabase/query_processor/util/tag_referenced_cards.clj new file mode 100644 index 0000000000000000000000000000000000000000..8f7c2979a3d3c61e79c4d5a110f9b2115c421f6f --- /dev/null +++ b/src/metabase/query_processor/util/tag_referenced_cards.clj @@ -0,0 +1,25 @@ +(ns metabase.query-processor.util.tag-referenced-cards + (:require + [metabase.models.card :refer [Card]] + [metabase.util.i18n :refer [tru]] + [toucan.db :as db])) + +(defn- query->template-tags + [query] + (vals (get-in query [:native :template-tags]))) + +(defn query->tag-card-ids + "Returns the card IDs from the template tags of the native query of `query`." + [query] + (keep :card-id (query->template-tags query))) + +(defn tags-referenced-cards + "Returns Card instances referenced by the given native `query`." + [query] + (mapv + (fn [card-id] + (if-let [card (db/select-one Card :id card-id)] + card + (throw (ex-info (tru "Referenced question #{0} could not be found" (str card-id)) + {:card-id card-id})))) + (query->tag-card-ids query))) diff --git a/test/metabase/query_processor/middleware/resolve_referenced_test.clj b/test/metabase/query_processor/middleware/resolve_referenced_test.clj index 15bf1dfbbe666cf0ef3b83689ee531f562fce714..958f30a469d24c9756a0ae875c470b3cff27eadd 100644 --- a/test/metabase/query_processor/middleware/resolve_referenced_test.clj +++ b/test/metabase/query_processor/middleware/resolve_referenced_test.clj @@ -15,19 +15,6 @@ (set! *warn-on-reflection* true) -(deftest tags-referenced-cards-lookup-test - (testing "returns Card instances from raw query" - (mt/with-temp* [Card [c1 {}] - Card [c2 {}]] - (is (= [c1 c2] - (#'qp.resolve-referenced/tags-referenced-cards - {:native - {:template-tags - {"tag-name-not-important1" {:type :card - :card-id (:id c1)} - "tag-name-not-important2" {:type :card - :card-id (:id c2)}}}})))))) - (deftest resolve-card-resources-test (testing "resolve stores source table from referenced card" (mt/with-temp Card [mbql-card {:dataset_query (mt/mbql-query venues diff --git a/test/metabase/query_processor/util/tag_referenced_cards_test.clj b/test/metabase/query_processor/util/tag_referenced_cards_test.clj new file mode 100644 index 0000000000000000000000000000000000000000..4773002ec97490b7a38cc0131102e48b04b467eb --- /dev/null +++ b/test/metabase/query_processor/util/tag_referenced_cards_test.clj @@ -0,0 +1,19 @@ +(ns metabase.query-processor.util.tag-referenced-cards-test + (:require + [clojure.test :refer :all] + [metabase.models.card :refer [Card]] + [metabase.query-processor.util.tag-referenced-cards :as qp.u.tag-referenced-cards] + [metabase.test :as mt])) + +(deftest tags-referenced-cards-lookup-test + (testing "returns Card instances from raw query" + (mt/with-temp* [Card [c1 {}] + Card [c2 {}]] + (is (= [c1 c2] + (qp.u.tag-referenced-cards/tags-referenced-cards + {:native + {:template-tags + {"tag-name-not-important1" {:type :card + :card-id (:id c1)} + "tag-name-not-important2" {:type :card + :card-id (:id c2)}}}})))))) diff --git a/test/metabase/query_processor_test/native_test.clj b/test/metabase/query_processor_test/native_test.clj index 56093e4d0f849b08a6b132fad98f89878c858a69..9d0a88b8e924cb8fd81d836a7f758f652329b943 100644 --- a/test/metabase/query_processor_test/native_test.clj +++ b/test/metabase/query_processor_test/native_test.clj @@ -1,9 +1,11 @@ (ns metabase.query-processor-test.native-test (:require [clojure.test :refer :all] + [metabase.models.card :refer [Card]] [metabase.query-processor :as qp] [metabase.query-processor-test :as qp.test] - [metabase.test :as mt])) + [metabase.test :as mt] + [metabase.util :as u])) (deftest native-test (is (= {:rows @@ -33,3 +35,22 @@ (qp/process-query (mt/native-query {:query "select name from users;"})))))) + +(deftest native-referring-question-referring-question-test + (testing "Should be able to run native query referring a question referring a question (#25988)" + (mt/with-driver :h2 + (mt/dataset sample-dataset + (mt/with-temp* [Card [card1 {:dataset_query (mt/mbql-query products)}] + Card [card2 {:dataset_query {:query {:source-table (str "card__" (u/the-id card1))} + :database (u/the-id (mt/db)) + :type :query}}]] + (let [card-tag (str "#" (u/the-id card2)) + query {:query (format "SELECT CATEGORY, VENDOR FROM {{%s}} ORDER BY ID LIMIT 1" card-tag) + :template-tags {card-tag + {:id "afd1bf85-61d0-258c-99a7-a5b448728308" + :name card-tag + :display-name card-tag + :type :card + :card-id (u/the-id card2)}}}] + (is (= [["Gizmo" "Swaniawski, Casper and Hilll"]] + (mt/rows (qp/process-query (mt/native-query query)))))))))))