diff --git a/src/metabase/api/dataset.clj b/src/metabase/api/dataset.clj
index 3f8651b75284ade90938ba04aca5e6ee2b947c73..13a6ba9113a34517530749fe0025b998269b6c4d 100644
--- a/src/metabase/api/dataset.clj
+++ b/src/metabase/api/dataset.clj
@@ -12,11 +12,11 @@
[metabase.query-processor :as qp]
[metabase.util :as u]))
-(def ^:const api-max-results-bare-rows
+(def ^:private ^:const api-max-results-bare-rows
"Maximum number of rows to return specifically on :rows type queries via the API."
2000)
-(def ^:const api-max-results
+(def ^:private ^:const api-max-results
"General maximum number of rows to return from an API query."
10000)
@@ -65,6 +65,7 @@
:body (:error response)})))
+;; TODO - AFAIK this endpoint is no longer used. Remove it? </3
(defendpoint GET "/card/:id"
"Execute the MQL query for a given `Card` and retrieve both the `Card` and the execution results as JSON.
This is a convenience endpoint which simplifies the normal 2 api calls to fetch the `Card` then execute its query."
diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj
index 1796785611494937e53851d84c7b37491c2d2d91..a7c779e3fdd2c8d852b8ac641751975a4c74d8c4 100644
--- a/src/metabase/driver.clj
+++ b/src/metabase/driver.clj
@@ -147,14 +147,18 @@
(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
+ 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.
+ 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 \"SELECT * FROM my_table\"}")
+ {:query \"-- [Contents of `(query->remark query)`]
+ SELECT * FROM my_table\"}")
(notify-database-updated [this, ^DatabaseInstance database]
"*OPTIONAL*. Notify the driver that the attributes of the DATABASE have changed. This is specifically relevant in
diff --git a/src/metabase/driver/bigquery.clj b/src/metabase/driver/bigquery.clj
index 0736d57ab407a8ef91d821a41c815a0acfd0cab2..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])
@@ -336,7 +337,7 @@
(binding [sqlqp/*query* outer-query]
(let [honeysql-form (honeysql-form outer-query)
sql (honeysql-form->sql honeysql-form)]
- {:query sql
+ {:query (str "-- " (qp/query->remark outer-query) "\n" sql)
:table-name table-name
:mbql? true})))
diff --git a/src/metabase/driver/generic_sql/query_processor.clj b/src/metabase/driver/generic_sql/query_processor.clj
index 45b78f3c4801f2cc93fe3803910428d91a61772e..1acb0201b0084b8ee74228131af659b58699efd6 100644
--- a/src/metabase/driver/generic_sql/query_processor.clj
+++ b/src/metabase/driver/generic_sql/query_processor.clj
@@ -276,7 +276,7 @@
(binding [*query* outer-query]
(let [honeysql-form (build-honeysql-form driver outer-query)
[sql & args] (sql/honeysql-form->sql+args driver honeysql-form)]
- {:query 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 cdbd52db2c702c55dc84ef11092b23e856b91e3f..a6431d97988684bd20d3c25777e7e5359a980605 100644
--- a/src/metabase/driver/mongo/query_processor.clj
+++ b/src/metabase/driver/mongo/query_processor.clj
@@ -326,7 +326,9 @@
;;; # process + run
-(defn- generate-aggregation-pipeline [query]
+(defn- generate-aggregation-pipeline
+ "Generate the aggregation pipeline. Returns a sequence of maps representing each stage."
+ [query]
(loop [pipeline [], [f & more] [add-initial-projection
handle-filter
handle-breakout+aggregation
diff --git a/src/metabase/query_processor.clj b/src/metabase/query_processor.clj
index 7f88e3072af82504f925cd1f45fbca6b78b15a96..a66860d3479c279acb4d98c214f0e1c35847a21f 100644
--- a/src/metabase/query_processor.clj
+++ b/src/metabase/query_processor.clj
@@ -428,6 +428,12 @@
(assert (sequential? (:columns <>)))
(assert (every? u/string-or-keyword? (:columns <>))))))))
+(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 [{{: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.
@@ -495,23 +501,22 @@
;; 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)]
- ((<<- 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)))))
;;; +----------------------------------------------------------------------------------------------------+
@@ -520,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.
@@ -533,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
@@ -552,15 +565,15 @@
: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)]
- (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))
+ (let [result (process-query query)]
+ (assert-valid-query-result result)
+ (query-complete query-execution result))
(catch Throwable e
(log/error (u/format-color 'red "Query failure: %s" (.getMessage e)))
(query-fail query-execution (.getMessage e))))))
@@ -576,7 +589,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
@@ -596,7 +609,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)))