Don't analyse non-SQL cards with Macaw (#44350)

src/metabase/driver/sql.clj

@@ -64,7 +64,7 @@
            :params params)))
 
 ;; `:sql` drivers almost certainly don't need to override this method, and instead can implement
-;; `unprepare/unprepare-value` for specific classes, or, in extereme cases, `unprepare/unprepare` itself.
+;; `unprepare/unprepare-value` for specific classes, or, in extreme cases, `unprepare/unprepare` itself.
 (defmethod driver/splice-parameters-into-native-query :sql
   [driver {:keys [params], sql :query, :as query}]
   (cond-> query

src/metabase/models/query_field.clj

@@ -24,7 +24,7 @@
   [query]
   (case (lib/normalized-query-type query)
     :native     (try
-                  (query-analyzer/field-ids-for-sql query)
+                  (query-analyzer/field-ids-for-native query)
                   (catch Exception e
                     (log/error e "Error parsing SQL" query)))
     :query      {:explicit (mbql.u/referenced-field-ids query)}
@@ -47,7 +47,7 @@
           query-field-rows                    (concat
                                                (map (partial id->row true) explicit)
                                                (map (partial id->row false) implicit))]
-      ;; when response is `nil`, it's a disabled parser, not unknown columns
+      ;; when the response is `nil`, it's a disabled parser, not unknown columns
       (when (some? res)
         (t2/with-transaction [_conn]
           (let [existing            (t2/select :model/QueryField :card_id card-id)
@@ -57,7 +57,7 @@
                                                 {:id-fn      :field_id
                                                  :to-compare #(dissoc % :id :card_id :field_id)})]
             (when (seq to-delete)
-              ;; this delete seems to break transaction (implicit commit or something) on MySQL, and this `diff`
+              ;; this deletion seems to break transaction (implicit commit or something) on MySQL, and this `diff`
               ;; algo drops its frequency by a lot - which should help with transactions affecting each other a
               ;; lot. Parallel tests in `metabase.models.query.permissions-test` were breaking when delete was
               ;; executed unconditionally on every query change.

src/metabase/native_query_analyzer.clj

@@ -15,6 +15,7 @@
    [clojure.string :as str]
    [macaw.core :as macaw]
    [metabase.config :as config]
+   [metabase.driver :as driver]
    [metabase.driver.util :as driver.u]
    [metabase.native-query-analyzer.impl :as nqa.impl]
    [metabase.native-query-analyzer.parameter-substitution :as nqa.sub]
@@ -138,24 +139,32 @@
                                                     [:= :f.active true]
                                                     (into [:or] (map table-query tables))]}))))
 
-(defn field-ids-for-sql
+(defn- field-ids-for-sql
   "Returns a `{:explicit #{...} :implicit #{...}}` map with field IDs that (may) be referenced in the given card's
   query. Errs on the side of optimism: i.e., it may return fields that are *not* in the query, and is unlikely to fail
   to return fields that are in the query.
 
   Explicit references are columns that are named in the query; implicit ones are from wildcards. If a field could be
   both explicit and implicit, it will *only* show up in the `:explicit` set."
+  [driver query]
+  (let [db-id        (:database query)
+        macaw-opts   (nqa.impl/macaw-options driver)
+        sql-string   (:query (nqa.sub/replace-tags query))
+        parsed-query (macaw/query->components (macaw/parsed-query sql-string) macaw-opts)
+        explicit-ids (explicit-field-ids-for-query parsed-query db-id)
+        implicit-ids (set/difference
+                      (implicit-field-ids-for-query parsed-query db-id)
+                      explicit-ids)]
+    {:explicit   explicit-ids
+     :implicit implicit-ids}))
+
+(defn field-ids-for-native
+  "Returns a `{:explicit #{...} :implicit #{...}}` map with field IDs that (may) be referenced in the given card's
+  query. Currently only support SQL-based dialects."
   [query]
-  (when (and (active?)
-             (:native query))
-    (let [db-id        (:database query)
-          sql-string   (:query (nqa.sub/replace-tags query))
-          driver       (driver.u/database->driver db-id)
-          macaw-opts   (nqa.impl/macaw-options driver)
-          parsed-query (macaw/query->components (macaw/parsed-query sql-string) macaw-opts)
-          explicit-ids (explicit-field-ids-for-query parsed-query db-id)
-          implicit-ids (set/difference
-                        (implicit-field-ids-for-query parsed-query db-id)
-                        explicit-ids)]
-      {:explicit   explicit-ids
-       :implicit implicit-ids})))
+  (when (and (active?) (:native query))
+    (let [driver (driver.u/database->driver (:database query))]
+      ;; TODO this approach is not extensible, we need to move to multimethods.
+      ;; See https://github.com/metabase/metabase/issues/43516 for long term solution.
+      (when (isa? driver/hierarchy driver :sql)
+        (field-ids-for-sql driver query)))))

test/metabase/native_query_analyzer_test.clj

@@ -33,7 +33,7 @@
 (deftest ^:parallel field-matching-test
   (binding [query-analyzer/*parse-queries-in-test?* true]
     (let [q (fn [sql]
-              (#'query-analyzer/field-ids-for-sql (mt/native-query {:query sql})))]
+              (#'query-analyzer/field-ids-for-native (mt/native-query {:query sql})))]
       (testing "simple query matches"
         (is (= {:explicit #{(mt/id :venues :id)} :implicit nil}
                (q "select id from venues"))))