diff --git a/src/metabase/query_analysis.clj b/src/metabase/query_analysis.clj
index d09e578f012d5ef7cc434a747036624712203404..09fa1a72f9ff2cda269d968e0750024f02d39d9d 100644
--- a/src/metabase/query_analysis.clj
+++ b/src/metabase/query_analysis.clj
@@ -108,7 +108,8 @@
:native (try
(nqa/references-for-native query)
(catch Exception e
- (log/debug e "Failed to analyze native query" query)))
+ (log/debug e "Failed to analyze native query" query)
+ {:error :query-analysis.error/exception, :context {:exception e}}))
;; For now, all model references are resolved transitively to the ultimate field ids.
;; We may want to change to record model references directly rather than resolving them.
;; This would remove the need to invalidate consuming cards when a given model changes.
@@ -124,36 +125,38 @@
(let [query-type (lib/normalized-query-type query)]
(when (enabled-type? query-type)
(t2/with-transaction [_conn]
- (let [analysis-id (t2/insert-returning-pk! :model/QueryAnalysis {:card_id card-id :status "running"})
- references (query-references query query-type)
- table->row (fn [{:keys [schema table table-id]}]
- {:card_id card-id
- :analysis_id analysis-id
- :schema schema
- :table table
- :table_id table-id})
- field->row (fn [{:keys [schema table column table-id field-id explicit-reference]}]
- {:card_id card-id
- :analysis_id analysis-id
- :schema schema
- :table table
- :column column
- :table_id table-id
- :field_id field-id
- :explicit_reference explicit-reference})
- success? (some? references)]
-
- (if-not success?
+ (let [analysis-id (t2/insert-returning-pk! :model/QueryAnalysis {:card_id card-id :status "running"})
+ result (query-references query query-type)
+ table->row (fn [{:keys [schema table table-id]}]
+ {:card_id card-id
+ :analysis_id analysis-id
+ :schema schema
+ :table table
+ :table_id table-id})
+ field->row (fn [{:keys [schema table column table-id field-id explicit-reference]}]
+ {:card_id card-id
+ :analysis_id analysis-id
+ :schema schema
+ :table table
+ :column column
+ :table_id table-id
+ :field_id field-id
+ :explicit_reference explicit-reference})]
+
+ (if (contains? result :error)
+ ;; TODO we should track cases where the driver is disabled or not-supported differently.
(t2/update! :model/QueryAnalysis analysis-id {:status "failed"})
(do
- (t2/insert! :model/QueryField (map field->row (:fields references)))
- (t2/insert! :model/QueryTable (map table->row (:tables references)))
+ (t2/insert! :model/QueryField (map field->row (:fields result)))
+ (t2/insert! :model/QueryTable (map table->row (:tables result)))
(t2/update! :model/QueryAnalysis analysis-id {:status "complete"})))
(t2/delete! :model/QueryAnalysis
{:where [:and
[:= :card_id card-id]
- [:not= :id analysis-id]]}))))))
+ [:not= :id analysis-id]]})
+
+ result)))))
(defn- replaced-inner-query-for-native-card
"Substitute new references for certain fields and tables, based upon the given mappings."
diff --git a/src/metabase/query_analysis/native_query_analyzer.clj b/src/metabase/query_analysis/native_query_analyzer.clj
index 975e746cbbd4a0c95f82fa145d51d331b578feae..daadac31156250bef3545c91e4773677d01e164c 100644
--- a/src/metabase/query_analysis/native_query_analyzer.clj
+++ b/src/metabase/query_analysis/native_query_analyzer.clj
@@ -335,8 +335,9 @@
;; For now we are not restricting this as we are for [[tables-for-native]], yet.
;; As we don't have any hard dependencies on this data, its useful to gather info on which drivers have errors or
;; inaccuracies in practice.
- (when (isa? driver/hierarchy driver :sql)
- (references-for-sql driver query))))
+ (if (isa? driver/hierarchy driver :sql)
+ (references-for-sql driver query)
+ {:error :query-analysis.error/driver-not-supported})))
(defn tables-for-native
"Returns a set of table identifiers that (may) be referenced in the given card's query.
diff --git a/src/metabase/task/analyze_queries.clj b/src/metabase/task/analyze_queries.clj
index 222f849f3525d46f419c77d4861e2c491efc25be..6a8d41eb97cadac933c94370af6fc92fec09a2ca 100644
--- a/src/metabase/task/analyze_queries.clj
+++ b/src/metabase/task/analyze_queries.clj
@@ -48,8 +48,9 @@
(if (failure-map/non-retryable? card)
(log/warnf "Skipping analysis of Card %s as its query has caused failures in the past." card-id)
(try
- (query-analysis/analyze!* card)
- (failure-map/track-success! card)
+ (if (:error (query-analysis/analyze!* card))
+ (failure-map/track-failure! card)
+ (failure-map/track-success! card))
(let [taken-ms (Math/ceil (u/since-ms timer))
sleep-ms (wait-proportional taken-ms)]
(log/debugf "Query analysis for Card %s took %sms (incl. persisting)" card-id taken-ms)
diff --git a/test/metabase/query_analysis/native_query_analyzer_test.clj b/test/metabase/query_analysis/native_query_analyzer_test.clj
index 671b35e827f1e359b9f4d9e2f0efd7212a99bbd3..b0083d88fee30d534cf9e1e38cb01257d1509fd1 100644
--- a/test/metabase/query_analysis/native_query_analyzer_test.clj
+++ b/test/metabase/query_analysis/native_query_analyzer_test.clj
@@ -41,8 +41,10 @@
(defn- refs [sql]
(-> (mt/native-query {:query sql})
(#'nqa/references-for-native)
+ (select-keys [:fields :tables])
(update-vals
- (partial sort-by (juxt :schema :table :column)))))
+ (partial sort-by (juxt :schema :table :column)))
+ not-empty))
(defn- field-refs [sql]
(:fields (refs sql)))
diff --git a/test/metabase/task/setup/query_analysis_setup.clj b/test/metabase/task/setup/query_analysis_setup.clj
index a415efa7f8c3d852e23240514eec3fb4367b1f90..cc3d5506a42e92bdaf9578ac8b8926865288d891 100644
--- a/test/metabase/task/setup/query_analysis_setup.clj
+++ b/test/metabase/task/setup/query_analysis_setup.clj
@@ -17,34 +17,35 @@
mlv2-query (-> (lib/query metadata-provider venues)
(lib/aggregate (lib/distinct venues-name)))]
- (mt/with-temp [Card c1 {:query_type "native"
- :dataset_query (mt/native-query {:query "SELECT id FROM venues"})}
- Card c2 {:query_type "native"
- :dataset_query (mt/native-query {:query "SELECT id FROM venues WHERE name = {{ name }}"
- :template-tags {"name" {:id "_name_"
- :type :text
- :display-name "name"
- :default "qwe"}}})}
- Card c3 {:query_type "query"
- :dataset_query (mt/mbql-query venues {:aggregation [[:distinct $name]]})}
- Card c4 {:query_type "query"
- :dataset_query mlv2-query}
- Card archived {:archived true
- :query_type "native"
- :dataset_query (mt/native-query {:query "SELECT id FROM venues"})}
- Card invalid {:query_type "native"
- :dataset_query (mt/native-query {:query "SELECT boom, FROM"})}]
+ (mt/with-temporary-setting-values [query-analysis-enabled true]
+ (mt/with-temp [Card c1 {:query_type "native"
+ :dataset_query (mt/native-query {:query "SELECT id FROM venues"})}
+ Card c2 {:query_type "native"
+ :dataset_query (mt/native-query {:query "SELECT id FROM venues WHERE name = {{ name }}"
+ :template-tags {"name" {:id "_name_"
+ :type :text
+ :display-name "name"
+ :default "qwe"}}})}
+ Card c3 {:query_type "query"
+ :dataset_query (mt/mbql-query venues {:aggregation [[:distinct $name]]})}
+ Card c4 {:query_type "query"
+ :dataset_query mlv2-query}
+ Card archived {:archived true
+ :query_type "native"
+ :dataset_query (mt/native-query {:query "SELECT id FROM venues"})}
+ Card invalid {:query_type "native"
+ :dataset_query (mt/native-query {:query "SELECT boom, FROM"})}]
- ;; Make sure there is no existing analysis for the relevant cards
- (t2/delete! :model/QueryAnalysis :card_id [:in (map :id [c1 c2 c3 c4 archived invalid])])
+ ;; Make sure there is no existing analysis for the relevant cards
+ (t2/delete! :model/QueryAnalysis :card_id [:in (map :id [c1 c2 c3 c4 archived invalid])])
- ;; Make sure some other card has analysis
- (query-analysis/analyze!* c3)
+ ;; Make sure some other card has analysis
+ (query-analysis/analyze!* c3)
- ;; And attempt to analyze an invalid query
- (query-analysis/analyze!* invalid)
+ ;; And attempt to analyze an invalid query
+ (query-analysis/analyze!* invalid)
- (mt/call-with-map-params f [c1 c2 c3 c4 archived invalid]))))
+ (mt/call-with-map-params f [c1 c2 c3 c4 archived invalid])))))
(defmacro with-test-setup!
"Set up the data required to test the Query Analysis related tasks"