diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj
index 52347143d180c4cb7d8b567228550e3b9d29946b..a7c779e3fdd2c8d852b8ac641751975a4c74d8c4 100644
--- a/src/metabase/driver.clj
+++ b/src/metabase/driver.clj
@@ -144,20 +144,20 @@
"*OPTIONAL*. Return a humanized (user-facing) version of an connection error message string.
Generic error messages are provided in the constant `connection-error-messages`; return one of these whenever possible.")
- (mbql->native ^java.util.Map [this, ^Map query, ^String remark]
+ (mbql->native ^java.util.Map [this, ^Map query]
"Transpile an MBQL structured query into the appropriate native query form.
The input QUERY will be a [fully-expanded MBQL query](https://github.com/metabase/metabase/wiki/Expanded-Queries) with
all the necessary pieces of information to build a properly formatted native query for the given database.
- REMARK contains general information about the query and user who ran it; if possible, a driver should include this at the begging of the native form
- so as to help DBAs identify queries originating from Metabase (see [issue #2386](https://github.com/metabase/metabase/issues/2386) for further discussion).
+ If the underlying query language supports remarks or comments, the driver should use `query->remark` to generate an appropriate message and include that in an appropriate place;
+ alternatively a driver might directly include the query's `:info` dictionary if the underlying language is JSON-based.
The result of this function will be passed directly into calls to `execute-query`.
For example, a driver like Postgres would build a valid SQL expression and return a map such as:
- {:query \"-- [Contents of `remark`]
+ {:query \"-- [Contents of `(query->remark query)`]
SELECT * FROM my_table\"}")
(notify-database-updated [this, ^DatabaseInstance database]
diff --git a/src/metabase/driver/bigquery.clj b/src/metabase/driver/bigquery.clj
index c170945801db1e01096fe1de26c4b39af64c70bd..48bf5e41d1f76436577582ba40aa5e711a5d9a6f 100644
--- a/src/metabase/driver/bigquery.clj
+++ b/src/metabase/driver/bigquery.clj
@@ -15,6 +15,7 @@
[field :as field]
[table :as table])
[metabase.sync-database.analyze :as analyze]
+ [metabase.query-processor :as qp]
metabase.query-processor.interface
[metabase.util :as u]
[metabase.util.honeysql-extensions :as hx])
@@ -331,12 +332,12 @@
(for [row rows]
(zipmap columns row))))
-(defn- mbql->native [{{{:keys [dataset-id]} :details, :as database} :database, {{table-name :name} :source-table} :query, :as outer-query} remark]
+(defn- mbql->native [{{{:keys [dataset-id]} :details, :as database} :database, {{table-name :name} :source-table} :query, :as outer-query}]
{:pre [(map? database) (seq dataset-id) (seq table-name)]}
(binding [sqlqp/*query* outer-query]
(let [honeysql-form (honeysql-form outer-query)
sql (honeysql-form->sql honeysql-form)]
- {:query (str "-- " remark "\n" sql)
+ {:query (str "-- " (qp/query->remark outer-query) "\n" sql)
:table-name table-name
:mbql? true})))
diff --git a/src/metabase/driver/druid/query_processor.clj b/src/metabase/driver/druid/query_processor.clj
index 30eb8464d0dc795cedc0a6eb3ed72f2aee97e182..d285fbf1cb67836c46cc44fe2508d6b5bd6b83da 100644
--- a/src/metabase/driver/druid/query_processor.clj
+++ b/src/metabase/driver/druid/query_processor.clj
@@ -521,7 +521,7 @@
(defn mbql->native
"Transpile an MBQL (inner) query into a native form suitable for a Druid DB."
- [query remark]
+ [query]
;; Merge `:settings` into the inner query dict so the QP has access to it
(let [mbql-query (assoc (:query query)
:settings (:settings query))]
diff --git a/src/metabase/driver/generic_sql/query_processor.clj b/src/metabase/driver/generic_sql/query_processor.clj
index c82e1285004025b1889cb1fa5adbf531282ea57b..1acb0201b0084b8ee74228131af659b58699efd6 100644
--- a/src/metabase/driver/generic_sql/query_processor.clj
+++ b/src/metabase/driver/generic_sql/query_processor.clj
@@ -272,11 +272,11 @@
(defn mbql->native
"Transpile MBQL query into a native SQL statement."
- [driver {inner-query :query, database :database, :as outer-query} remark]
+ [driver {inner-query :query, database :database, :as outer-query}]
(binding [*query* outer-query]
(let [honeysql-form (build-honeysql-form driver outer-query)
[sql & args] (sql/honeysql-form->sql+args driver honeysql-form)]
- {:query (str "-- " remark "\n" sql)
+ {:query (str "-- " (qp/query->remark outer-query) "\n" sql)
:params args})))
diff --git a/src/metabase/driver/mongo/query_processor.clj b/src/metabase/driver/mongo/query_processor.clj
index 7c57c8d9d59025b2d0ef43db8d3f9e26acffea55..a6431d97988684bd20d3c25777e7e5359a980605 100644
--- a/src/metabase/driver/mongo/query_processor.clj
+++ b/src/metabase/driver/mongo/query_processor.clj
@@ -380,7 +380,7 @@
(defn mbql->native
"Process and run an MBQL query."
- [{database :database, {{source-table-name :name} :source-table} :query, :as query} remark]
+ [{database :database, {{source-table-name :name} :source-table} :query, :as query}]
{:pre [(map? database)
(string? source-table-name)]}
(binding [*query* query]
diff --git a/src/metabase/query_processor.clj b/src/metabase/query_processor.clj
index ad526cfb9659f663e066120eb279cf45176bdb0a..46c07573114f77743fc0e9d316e52dd6b6c7e347 100644
--- a/src/metabase/query_processor.clj
+++ b/src/metabase/query_processor.clj
@@ -428,15 +428,12 @@
(assert (sequential? (:columns <>)))
(assert (every? u/string-or-keyword? (:columns <>))))))))
-(defn- query->remark
+(defn query->remark
"Genarate an approparite REMARK to be prepended to a query to give DBAs additional information about the query being executed.
See documentation for `mbql->native` and [issue #2386](https://github.com/metabase/metabase/issues/2386) for more information."
- ^String [query query-execution]
- (let [executor-id (:executor_id query-execution)
- execution-id (:uuid query-execution)
- query-type (if (mbql-query? query) "MBQL" "native")
- query-hash (hash query)]
- (format "Metabase:: userID: %s executionID: %s queryType: %s queryHash: %s" executor-id execution-id query-type query-hash)))
+ ^String [{{:keys [executed-by uuid query-hash query-type]} :info, :as info}]
+ {:pre [(map? info)]}
+ (format "Metabase:: userID: %s executionID: %s queryType: %s queryHash: %s" executed-by uuid query-type query-hash))
(defn- run-query
"The end of the QP middleware which actually executes the query on the driver.
@@ -445,10 +442,10 @@
then we pass that form into the `execute-query` function for final execution.
If the query is already a *native* query then we simply pass it through to `execute-query` unmodified."
- [query query-execution]
+ [query]
(let [native-form (u/prog1 (if-not (mbql-query? query)
(:native query)
- (driver/mbql->native (:driver query) query (query->remark query query-execution)))
+ (driver/mbql->native (:driver query) query))
(when-not *disable-qp-logging*
(log/debug (u/format-color 'green "NATIVE FORM:\n%s\n" (u/pprint-to-str <>)))))
native-query (if-not (mbql-query? query)
@@ -498,30 +495,28 @@
(defn process-query
"Process an MBQL structured or native query, and return the result."
- [query & [query-execution]]
+ [query]
(when-not *disable-qp-logging*
(log/debug (u/format-color 'blue "\nQUERY: 😎\n%s" (u/pprint-to-str query))))
;; TODO: it probably makes sense to throw an error or return a failure response here if we can't get a driver
(let [driver (driver/database-id->driver (:database query))]
(binding [*driver* driver]
- (let [driver-process-in-context (partial driver/process-query-in-context driver)
- run-query (u/rpartial run-query query-execution)]
- ((<<- wrap-catch-exceptions
- pre-add-settings
- pre-expand-macros
- pre-expand-resolve
- driver-process-in-context
- post-add-row-count-and-status
- post-format-rows
- pre-add-implicit-fields
- pre-add-implicit-breakout-order-by
- cumulative-sum
- cumulative-count
- limit
- post-check-results-format
- pre-log-query
- guard-multiple-calls
- run-query) (assoc query :driver driver))))))
+ ((<<- wrap-catch-exceptions
+ pre-add-settings
+ pre-expand-macros
+ pre-expand-resolve
+ (driver/process-query-in-context driver)
+ post-add-row-count-and-status
+ post-format-rows
+ pre-add-implicit-fields
+ pre-add-implicit-breakout-order-by
+ cumulative-sum
+ cumulative-count
+ limit
+ post-check-results-format
+ pre-log-query
+ guard-multiple-calls
+ run-query) (assoc query :driver driver)))))
;;; +----------------------------------------------------------------------------------------------------+
@@ -530,6 +525,13 @@
(declare query-fail query-complete save-query-execution)
+(defn- assert-valid-query-result [query-result]
+ (when-not (contains? query-result :status)
+ (throw (Exception. "invalid response from database driver. no :status provided")))
+ (when (= :failed (:status query-result))
+ (log/error (u/pprint-to-str 'red query-result))
+ (throw (Exception. (str (get query-result :error "general error"))))))
+
(defn dataset-query
"Process and run a json based dataset query and return results.
@@ -543,11 +545,12 @@
Possible caller-options include:
- :executed_by [int] (user_id of caller)"
+ :executed_by [int] (user_id of caller)"
{:arglists '([query options])}
[query {:keys [executed_by]}]
{:pre [(integer? executed_by)]}
- (let [query-execution {:uuid (.toString (java.util.UUID/randomUUID))
+ (let [query-uuid (.toString (java.util.UUID/randomUUID))
+ query-execution {:uuid query-uuid
:executor_id executed_by
:json_query query
:query_id nil
@@ -562,15 +565,14 @@
:result_data "{}"
:raw_query ""
:additional_info ""
- :start_time_millis (System/currentTimeMillis)}]
+ :start_time_millis (System/currentTimeMillis)}
+ query (assoc query :info {:executed-by executed_by
+ :uuid query-uuid
+ :query-hash (hash query)
+ :query-type (if (mbql-query? query) "MBQL" "native")})]
(try
- (let [query-result (process-query query query-execution)]
- (when-not (contains? query-result :status)
- (throw (Exception. "invalid response from database driver. no :status provided")))
- (when (= :failed (:status query-result))
- (log/error (u/pprint-to-str 'red query-result))
- (throw (Exception. (str (get query-result :error "general error")))))
- (query-complete query-execution query-result))
+ (query-complete query-execution (u/prog1 (process-query query)
+ (assert-valid-query-result <>)))
(catch Throwable e
(log/error (u/format-color 'red "Query failure: %s" (.getMessage e)))
(query-fail query-execution (.getMessage e))))))
@@ -586,7 +588,7 @@
(-> query-execution
(dissoc :start_time_millis)
(merge updates)
- (save-query-execution)
+ save-query-execution
(dissoc :raw_query :result_rows :version)
;; this is just for the response for clien
(assoc :error error-message
@@ -606,7 +608,7 @@
(:start_time_millis query-execution))
:result_rows (get query-result :row_count 0))
(dissoc :start_time_millis)
- (save-query-execution)
+ save-query-execution
;; at this point we've saved and we just need to massage things into our final response format
(dissoc :error :raw_query :result_rows :version)
(merge query-result)))