`zoom`, `fk-details`, `fk-filter` drills should be returned for a native query model (#35891)

src/metabase/lib/drill_thru.cljc

@@ -4,7 +4,7 @@
    [metabase.lib.drill-thru.common :as lib.drill-thru.common]
    [metabase.lib.drill-thru.distribution :as lib.drill-thru.distribution]
    [metabase.lib.drill-thru.fk-details :as lib.drill-thru.fk-details]
-   [metabase.lib.drill-thru.foreign-key :as lib.drill-thru.foreign-key]
+   [metabase.lib.drill-thru.fk-filter :as lib.drill-thru.fk-filter]
    [metabase.lib.drill-thru.object-details :as lib.drill-thru.object-details]
    [metabase.lib.drill-thru.pivot :as lib.drill-thru.pivot]
    [metabase.lib.drill-thru.pk :as lib.drill-thru.pk]
@@ -46,7 +46,7 @@
   `:return-drills-for-dimensions?` specifies which type we have."
   [{:f #'lib.drill-thru.distribution/distribution-drill,                         :return-drills-for-dimensions? true}
    {:f #'lib.drill-thru.column-filter/column-filter-drill,                       :return-drills-for-dimensions? true}
-   {:f #'lib.drill-thru.foreign-key/foreign-key-drill,                           :return-drills-for-dimensions? false}
+   {:f #'lib.drill-thru.fk-filter/fk-filter-drill,                               :return-drills-for-dimensions? false}
    {:f #'lib.drill-thru.object-details/object-detail-drill,                      :return-drills-for-dimensions? false}
    {:f #'lib.drill-thru.pivot/pivot-drill,                                       :return-drills-for-dimensions? false}
    {:f #'lib.drill-thru.quick-filter/quick-filter-drill,                         :return-drills-for-dimensions? false}

src/metabase/lib/drill_thru/fk_details.cljc

@@ -15,11 +15,11 @@
   (select-keys drill-thru [:many-pks? :object-id :type]))
 
 (defmethod lib.drill-thru.common/drill-thru-method :drill-thru/fk-details
-  [query _stage-number {:keys [column object-id]} & _]
+  [query stage-number {:keys [column object-id]} & _]
   ;; generate a NEW query against the FK target table and column, e.g. if the original query was
   ;; ORDERS/ORDERS.USER_ID, the new query should by PEOPLE/PEOPLE.ID.
-  (let [fk-column-id     (:fk-target-field-id column)
-        fk-column        (some->> fk-column-id (lib.metadata/field query))
-        fk-column-table  (some->> (:table-id fk-column) (lib.metadata/table query))]
+  (let [fk-column-id    (:fk-target-field-id column)
+        fk-column       (some->> fk-column-id (lib.metadata/field query))
+        fk-column-table (some->> (:table-id fk-column) (lib.metadata/table query))]
     (-> (lib.query/query query fk-column-table)
-        (lib.filter/filter (lib.filter/= fk-column object-id)))))
+        (lib.filter/filter stage-number (lib.filter/= fk-column object-id)))))

src/metabase/lib/drill_thru/fk_filter.cljc

+(ns metabase.lib.drill-thru.fk-filter
+  (:require
+   [metabase.lib.drill-thru.common :as lib.drill-thru.common]
+   [metabase.lib.filter :as lib.filter]
+   [metabase.lib.options :as lib.options]
+   [metabase.lib.ref :as lib.ref]
+   [metabase.lib.schema :as lib.schema]
+   [metabase.lib.schema.drill-thru :as lib.schema.drill-thru]
+   [metabase.lib.types.isa :as lib.types.isa]
+   [metabase.lib.util :as lib.util]
+   [metabase.util.malli :as mu]))
+
+(mu/defn fk-filter-drill :- [:maybe ::lib.schema.drill-thru/drill-thru.fk-filter]
+  "When clicking on a foreign key value, filter this query by that column.
+
+  This has the same effect as the `=` filter on a generic field (ie. not a key), but renders differently.
+
+  Contrast [[metabase.lib.drill-thru.object-details/object-detail-drill]], which shows the details of the foreign
+  object."
+  [_query                               :- ::lib.schema/query
+   _stage-number                        :- :int
+   {:keys [column value], :as _context} :- ::lib.schema.drill-thru/context]
+  (when (and column
+             (some? value)
+             (not= value :null) ; If the FK is null, don't show this option.
+             (not (lib.types.isa/primary-key? column))
+             (lib.types.isa/foreign-key? column))
+    {:lib/type :metabase.lib.drill-thru/drill-thru
+     :type     :drill-thru/fk-filter
+     :filter   (lib.options/ensure-uuid [:= {} (lib.ref/ref column) value])}))
+
+(mu/defmethod lib.drill-thru.common/drill-thru-method :drill-thru/fk-filter :- ::lib.schema/query
+  [query        :- ::lib.schema/query
+   stage-number :- :int
+   drill-thru   :- ::lib.schema.drill-thru/drill-thru.fk-filter
+   & _args]
+  ;; if the stage in question is an MBQL stage, we can simply add a `=` filter to it. If it's a native stage, we have
+  ;; to apply the drill to the stage after that stage, which will be an MBQL stage, adding it if needed (native stages
+  ;; are currently only allowed to be the first stage.)
+  (let [[query stage-number] (if (lib.drill-thru.common/mbql-stage? query stage-number)
+                               [query stage-number]
+                               ;; native stage
+                               (let [;; convert the stage number e.g. `-1` to the canonical non-relative stage number
+                                     stage-number      (lib.util/canonical-stage-index query stage-number)
+                                     ;; make sure the query has at least one MBQL stage after the native stage, which we
+                                     ;; know is the first stage.
+                                     query             (lib.util/ensure-mbql-final-stage query)
+                                     next-stage-number (lib.util/next-stage-number query stage-number)]
+                                 (assert (lib.util/query-stage query next-stage-number)
+                                         "Sanity check: there should be an additional stage by now")
+                                 [query next-stage-number]))]
+    (lib.filter/filter query stage-number (:filter drill-thru))))

src/metabase/lib/drill_thru/foreign_key.cljc

-(ns metabase.lib.drill-thru.foreign-key
-  (:require
-   [metabase.lib.drill-thru.common :as lib.drill-thru.common]
-   [metabase.lib.filter :as lib.filter]
-   [metabase.lib.options :as lib.options]
-   [metabase.lib.ref :as lib.ref]
-   [metabase.lib.schema :as lib.schema]
-   [metabase.lib.schema.drill-thru :as lib.schema.drill-thru]
-   [metabase.lib.types.isa :as lib.types.isa]
-   [metabase.util.malli :as mu]))
-
-(mu/defn foreign-key-drill :- [:maybe ::lib.schema.drill-thru/drill-thru.fk-filter]
-  "When clicking on a foreign key value, filter this query by that column.
-
-  This has the same effect as the `=` filter on a generic field (ie. not a key), but renders differently.
-
-  Contrast [[object-detail-drill]], which shows the details of the foreign object."
-  [query                                :- ::lib.schema/query
-   stage-number                         :- :int
-   {:keys [column value], :as _context} :- ::lib.schema.drill-thru/context]
-  (when (and (lib.drill-thru.common/mbql-stage? query stage-number)
-             column
-             (some? value)
-             (not= value :null) ; If the FK is null, don't show this option.
-             (not (lib.types.isa/primary-key? column))
-             (lib.types.isa/foreign-key? column))
-    {:lib/type  :metabase.lib.drill-thru/drill-thru
-     :type      :drill-thru/fk-filter
-     :filter    (lib.options/ensure-uuid [:= {} (lib.ref/ref column) value])}))
-
-(defmethod lib.drill-thru.common/drill-thru-method :drill-thru/fk-filter
-  [query stage-number drill-thru & _args]
-  (lib.filter/filter query stage-number (:filter drill-thru)))

src/metabase/lib/drill_thru/object_details.cljc

@@ -14,12 +14,21 @@
               :type      :drill-thru/pk
               :column    column
               :object-id value
-              :many-pks? many-pks?}]
+              :many-pks? many-pks?}
+        mbql-stage? (lib.drill-thru.common/mbql-stage? query stage-number)]
     (cond
-      (and (lib.types.isa/primary-key? column) many-pks?) (assoc base :type :drill-thru/pk)
+      (and (lib.types.isa/primary-key? column)
+           many-pks?
+           mbql-stage?)
+      (assoc base :type :drill-thru/pk)
+
       ;; TODO: Figure out clicked.extraData and the dashboard flow.
-      (lib.types.isa/primary-key? column)                 (assoc base :type :drill-thru/zoom)
-      (lib.types.isa/foreign-key? column)                 (assoc base :type :drill-thru/fk-details)
+      (lib.types.isa/primary-key? column)
+      (assoc base :type :drill-thru/zoom)
+
+      (lib.types.isa/foreign-key? column)
+      (assoc base :type :drill-thru/fk-details)
+
       (and (not many-pks?)
            (not-empty row)
            (empty? (lib.aggregation/aggregations query stage-number)))
@@ -41,12 +50,12 @@
                                          ::lib.schema.drill-thru/drill-thru.fk-details]]
   "When clicking a foreign key or primary key value, drill through to the details for that specific object.
 
-  Contrast [[foreign-key-drill]], which filters this query to only those rows with a specific value for a FK column."
+  Contrast [[metabase.lib.drill-thru.fk-filter/fk-filter-drill]], which filters this query to only those rows with a
+  specific value for a FK column."
   [query                              :- ::lib.schema/query
    stage-number                       :- :int
    {:keys [column value] :as context} :- ::lib.schema.drill-thru/context]
-  (when (and (lib.drill-thru.common/mbql-stage? query stage-number)
-             column
+  (when (and column
              (some? value))
     (object-detail-drill-for query stage-number context
                              (> (count (lib.metadata.calculation/primary-keys query)) 1))))

src/metabase/lib/native.cljc

@@ -5,7 +5,6 @@
    [clojure.string :as str]
    [medley.core :as m]
    [metabase.lib.metadata :as lib.metadata]
-   [metabase.lib.options :as lib.options]
    [metabase.lib.query :as lib.query]
    [metabase.lib.schema :as lib.schema]
    [metabase.lib.schema.common :as common]
@@ -165,11 +164,10 @@
     native-extras :- [:maybe ::native-extras]]
    (let [tags (extract-template-tags inner-query)]
      (-> (lib.query/query-with-stages metadata-providerable
-                                      [(-> {:lib/type           :mbql.stage/native
-                                            :lib/stage-metadata results-metadata
-                                            :template-tags      tags
-                                            :native             inner-query}
-                                           lib.options/ensure-uuid)])
+                                      [{:lib/type           :mbql.stage/native
+                                        :lib/stage-metadata results-metadata
+                                        :template-tags      tags
+                                        :native             inner-query}])
          (with-native-extras native-extras)))))
 
 (mu/defn with-different-database :- ::lib.schema/query

src/metabase/lib/util.cljc

@@ -279,16 +279,17 @@
   [query stage-number]
   (not (previous-stage-number query stage-number)))
 
-(defn next-stage-number
+(mu/defn next-stage-number :- [:maybe :int]
   "The index of the next stage, if there is one. `nil` if there is no next stage."
-  [{:keys [stages], :as _query} stage-number]
+  [{:keys [stages], :as _query} :- :map
+   stage-number                 :- :int]
   (let [stage-number (if (neg? stage-number)
                        (+ (count stages) stage-number)
                        stage-number)]
     (when (< (inc stage-number) (count stages))
       (inc stage-number))))
 
-(mu/defn query-stage :- ::lib.schema/stage
+(mu/defn query-stage :- [:maybe ::lib.schema/stage]
   "Fetch a specific `stage` of a query. This handles negative indices as well, e.g. `-1` will return the last stage of
   the query."
   [query        :- LegacyOrPMBQLQuery

test/metabase/lib/drill_thru/fk_details_test.cljc

 (ns metabase.lib.drill-thru.fk-details-test
   (:require
-   [clojure.test :refer [deftest testing]]
+   [clojure.test :refer [deftest is testing]]
+   [medley.core :as m]
+   [metabase.lib.core :as lib]
    [metabase.lib.drill-thru.test-util :as lib.drill-thru.tu]
-   [metabase.lib.test-metadata :as meta]))
+   [metabase.lib.test-metadata :as meta]
+   #?@(:cljs ([metabase.test-runner.assert-exprs.approximately-equal]))))
+
+#?(:cljs (comment metabase.test-runner.assert-exprs.approximately-equal/keep-me))
 
 (deftest ^:parallel returns-fk-details-test-1
   (lib.drill-thru.tu/test-returns-drill
@@ -67,3 +72,49 @@
                                    :filters      [[:= {}
                                                    [:field {} (meta/id :people :id)]
                                                    column-value]]}]}}))))
+
+(deftest ^:parallel fk-details-on-model-test
+  (testing "FK details drill should work correctly for native query models (#35689)"
+    (let [native-metadata   (fn [col]
+                              (-> col
+                                  (assoc :lib/source :source/native)
+                                  (dissoc :id :table-id)))
+          orders-id         (native-metadata (meta/field-metadata :orders :id))
+          orders-user-id    (native-metadata (meta/field-metadata :orders :user-id))
+          orders-product-id (native-metadata (meta/field-metadata :orders :product-id))
+          query             (lib/native-query meta/metadata-provider
+                                              "SELECT id, user_id, product_id FROM ORDERS LIMIT 10;"
+                                              {:lib/type :metadata/results
+                                               :columns  [orders-id orders-user-id orders-product-id]}
+                                              nil)
+          context           {:column     orders-user-id
+                             :column-ref (lib/ref orders-user-id)
+                             :value      1
+                             :row        [{:column     orders-id
+                                           :column-ref (lib/ref orders-id)
+                                           :value      6}
+                                          {:column     orders-user-id
+                                           :column-ref (lib/ref orders-user-id)
+                                           :value      1}
+                                          {:column     orders-product-id
+                                           :column-ref (lib/ref orders-product-id)
+                                           :value      60}]}
+          drills            (lib/available-drill-thrus query context)
+          fk-details-drill  (m/find-first #(= (:type %) :drill-thru/fk-details)
+                                          drills)]
+      (testing "Drill should be returned"
+        (is (=? {:lib/type  :metabase.lib.drill-thru/drill-thru
+                 :type      :drill-thru/fk-details
+                 :column    {:name "USER_ID"}
+                 :object-id 1
+                 :many-pks? false}
+                fk-details-drill)))
+      (when fk-details-drill
+        (testing "Drill application"
+          (testing "Should introduce another query stage"
+            (is (=? {:stages [{:lib/type :mbql.stage/mbql,
+                               :source-table (meta/id :people)
+                               :filters [[:= {}
+                                          [:field {} (meta/id :people :id)]
+                                          1]]}]}
+                    (lib/drill-thru query -1 fk-details-drill)))))))))

test/metabase/lib/drill_thru/foreign_key_test.cljc → test/metabase/lib/drill_thru/fk_filter_test.cljc

-(ns metabase.lib.drill-thru.foreign-key-test
+(ns metabase.lib.drill-thru.fk-filter-test
   (:require
    [clojure.test :refer [deftest is testing]]
+   [medley.core :as m]
    [metabase.lib.core :as lib]
    [metabase.lib.drill-thru.test-util :as lib.drill-thru.tu]
-   [metabase.util :as u]))
+   [metabase.lib.test-metadata :as meta]
+   [metabase.util :as u]
+   #?@(:cljs ([metabase.test-runner.assert-exprs.approximately-equal]))))
+
+#?(:cljs (comment metabase.test-runner.assert-exprs.approximately-equal/keep-me))
 
 (deftest ^:parallel returns-fk-filter-test-1
   (lib.drill-thru.tu/test-returns-drill
@@ -57,3 +62,57 @@
                      :drill-thru/fk-filter))
       (is (not (contains? (drill-types (assoc context :value :null))
                           :drill-thru/fk-filter))))))
+
+(deftest ^:parallel fk-filter-on-model-test
+  (testing "FK filter drill should work correctly for native query models (#35689)"
+    (let [native-metadata   (fn [col]
+                              (-> col
+                                  (assoc :lib/source :source/native)
+                                  (dissoc :id :table-id)))
+          orders-id         (native-metadata (meta/field-metadata :orders :id))
+          orders-user-id    (native-metadata (meta/field-metadata :orders :user-id))
+          orders-product-id (native-metadata (meta/field-metadata :orders :product-id))
+          query             (lib/native-query meta/metadata-provider
+                                              "SELECT id, user_id, product_id FROM ORDERS LIMIT 10;"
+                                              {:lib/type :metadata/results
+                                               :columns  [orders-id orders-user-id orders-product-id]}
+                                              nil)
+          context           {:column     orders-user-id
+                             :column-ref (lib/ref orders-user-id)
+                             :value      1
+                             :row        [{:column     orders-id
+                                           :column-ref (lib/ref orders-id)
+                                           :value      6}
+                                          {:column     orders-user-id
+                                           :column-ref (lib/ref orders-user-id)
+                                           :value      1}
+                                          {:column     orders-product-id
+                                           :column-ref (lib/ref orders-product-id)
+                                           :value      60}]}
+          drills            (lib/available-drill-thrus query context)
+          fk-filter-drill   (m/find-first #(= (:type %) :drill-thru/fk-filter)
+                                          drills)]
+      (testing "Drill should be returned"
+        (is (=? {:lib/type :metabase.lib.drill-thru/drill-thru
+                 :type     :drill-thru/fk-filter
+                 :filter   [:= {} [:field {} "USER_ID"] 1]}
+                fk-filter-drill)))
+      (when fk-filter-drill
+        (testing "Drill application"
+          (testing "Should introduce another query stage"
+            (is (=? {:lib/type :mbql/query
+                     :stages   [{:lib/type :mbql.stage/native,
+                                 :native   "SELECT id, user_id, product_id FROM ORDERS LIMIT 10;"}
+                                {:lib/type :mbql.stage/mbql,
+                                 :filters  [[:= {} [:field {} "USER_ID"] 1]]}]}
+                    (lib/drill-thru query -1 fk-filter-drill))))
+          (testing "Query already has another stage after the native stage -- add filter to existing stage"
+            (let [query (-> (lib/append-stage query)
+                            (lib/filter (lib/> 3 1)))]
+              (is (=? {:lib/type :mbql/query
+                       :stages   [{:lib/type :mbql.stage/native,
+                                   :native   "SELECT id, user_id, product_id FROM ORDERS LIMIT 10;"}
+                                  {:lib/type :mbql.stage/mbql,
+                                   :filters  [[:> {} 3 1]
+                                              [:= {} [:field {} "USER_ID"] 1]]}]}
+                      (lib/drill-thru query 0 fk-filter-drill))))))))))

test/metabase/lib/native_test.cljc

@@ -11,6 +11,8 @@
    [metabase.util.humanization :as u.humanization]
    [metabase.util.malli :as mu]))
 
+#?(:cljs (comment metabase.test-runner.assert-exprs.approximately-equal/keep-me))
+
 (deftest ^:parallel variable-tag-test
   (are [exp input] (= exp (set (keys (lib.native/extract-template-tags input))))
     #{"foo"} "SELECT * FROM table WHERE {{foo}} AND some_field IS NOT NULL"
@@ -148,9 +150,8 @@
 (deftest ^:parallel native-query-test
   (is (=? {:lib/type :mbql/query
            :database (meta/id)
-           :stages   [{:lib/type    :mbql.stage/native
-                       :lib/options {:lib/uuid string?}
-                       :native      "SELECT * FROM VENUES;"}]}
+           :stages   [{:lib/type :mbql.stage/native
+                       :native   "SELECT * FROM VENUES;"}]}
           (lib/native-query meta/metadata-provider "SELECT * FROM VENUES;" qp-results-metadata nil))))
 
 (deftest ^:parallel native-query-suggested-name-test