Resolve nested questions in native queries (#28511)

* Resolve nested questions in native queries

Fixes #25988.

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]

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*

src/metabase/query_processor/middleware/resolve_referenced.clj

 (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)

src/metabase/query_processor/util/tag_referenced_cards.clj

+(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)))

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

test/metabase/query_processor/util/tag_referenced_cards_test.clj

+(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)}}}}))))))

test/metabase/query_processor_test/native_test.clj

 (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)))))))))))