diff --git a/src/metabase/api/qs.clj b/src/metabase/api/qs.clj
index bfed2e0147d67d25c8f1a067d2b984fe45152b03..67a338a80326de6a7cdd48f82b0424ab1ccc53a9 100644
--- a/src/metabase/api/qs.clj
+++ b/src/metabase/api/qs.clj
@@ -10,8 +10,6 @@
[metabase.util :refer [contains-many? now-iso8601]]))
-(declare execute-query)
-
(defendpoint POST "/" [:as {{:keys [timezone database sql] :as body} :body}]
{database [Required Integer]
sql [Required NonEmptyString]} ; TODO - check timezone
diff --git a/src/metabase/api/query.clj b/src/metabase/api/query.clj
index 946109a13b15ecf138a7190a074784b691aa759a..44e0c12e2ac0d769247b7ab494599b2520ed2eeb 100644
--- a/src/metabase/api/query.clj
+++ b/src/metabase/api/query.clj
@@ -108,7 +108,7 @@
(let-404 [{database-id :database_id
{:keys [sql timezone]} :details :as query} (sel :one Query :id id)]
(read-check query)
- (let [dataset-query {:type "native"
+ (let [dataset-query {:type :native
:database database-id
:native {:query sql
:timezone timezone}}
diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj
index 8918540b6a54a99a3e85c994318f99db99f7cc91..0b84d9120ad1f48267eb7ad4d68847f84d729864 100644
--- a/src/metabase/driver.clj
+++ b/src/metabase/driver.clj
@@ -69,19 +69,20 @@
(defn dataset-query
"Process and run a json based dataset query and return results.
- Takes 2 arguments:
- 1. the json query as a dictionary
- 2. query execution options specified in a dictionary
-
- Depending on the database specified in the query this function will delegate to a driver specific implementation.
- For the purposes of tracking we record each call to this function as a QueryExecution in the database.
-
- Possible caller-options include:
- :executed_by [int] (user_id of caller)
- :saved_query [{}] (dictionary representing Query model)
- :synchronously [true|false] (default true)
- :cache_result [true|false] (default false)
- "
+ Takes 2 arguments:
+
+ 1. the json query as a dictionary
+ 2. query execution options specified in a dictionary
+
+ Depending on the database specified in the query this function will delegate to a driver specific implementation.
+ For the purposes of tracking we record each call to this function as a QueryExecution in the database.
+
+ Possible caller-options include:
+
+ :executed_by [int] (user_id of caller)
+ :saved_query [{}] (dictionary representing Query model)
+ :synchronously [true|false] (default true)
+ :cache_result [true|false] (default false)"
{:arglists '([query caller-options])}
[query {:keys [executed_by synchronously saved_query]
:or {synchronously true}
@@ -115,16 +116,17 @@
;; this ensures the currently saved query-execution is what gets returned
query-execution))))
-
(defn -dataset-query
"Execute a query and record the outcome. Entire execution is wrapped in a try-catch to prevent Exceptions
- from leaking outside the function call."
+ from leaking outside the function call."
[query options query-execution]
(let [query-execution (assoc query-execution :start_time_millis (System/currentTimeMillis))]
(try
(let [query-result (execute-query query)]
- (when-not (contains? query-result :status) (throw (Exception. "invalid response from database driver. no :status provided")))
- (when (= :failed (:status query-result)) (throw (Exception. ^String (get query-result :error "general error"))))
+ (when-not (contains? query-result :status)
+ (throw (Exception. "invalid response from database driver. no :status provided")))
+ (when (= :failed (:status query-result))
+ (throw (Exception. ^String (get query-result :error "general error"))))
(query-complete query-execution query-result (:cache_result options)))
(catch Exception ex
(log/warn ex)
@@ -140,14 +142,14 @@
:running_time (- (System/currentTimeMillis) (:start_time_millis query-execution))}]
;; record our query execution and format response
(-> query-execution
- (dissoc :start_time_millis)
- (merge updates)
- (save-query-execution)
- ;; this is just for the response for clien
- (assoc :row_count 0
- :data {:rows []
- :cols []
- :columns []}))))
+ (dissoc :start_time_millis)
+ (merge updates)
+ (save-query-execution)
+ ;; this is just for the response for clien
+ (assoc :row_count 0
+ :data {:rows []
+ :cols []
+ :columns []}))))
(defn query-complete
"Save QueryExecution state and construct a completed (successful) query response"
diff --git a/src/metabase/driver/generic_sql/native.clj b/src/metabase/driver/generic_sql/native.clj
index 50ba2b1706980698813f60c292954ec61fd4d908..70f762ce1046f263cd6c9eeb8e2abec08e600ba0 100644
--- a/src/metabase/driver/generic_sql/native.clj
+++ b/src/metabase/driver/generic_sql/native.clj
@@ -1,8 +1,10 @@
(ns metabase.driver.generic-sql.native
"The `native` query processor."
(:import com.metabase.corvus.api.ApiException)
- (:require [clojure.tools.logging :as log]
+ (:require [clojure.java.jdbc :as jdbc]
+ [clojure.tools.logging :as log]
[korma.core :as korma]
+ korma.db
[metabase.db :refer [sel]]
[metabase.driver.generic-sql.util :refer :all]
[metabase.models.database :refer [Database]]))
@@ -23,23 +25,28 @@
(or (class->base-type (type v))
(throw (ApiException. (int 500) (format "Missing base type mapping for %s in metabase.driver.native/class->base-type. Please add an entry."
(str (type v))))))))
-
-(defn- get-cols [row]
- (->> row
- (map (fn [[k v]]
- {:name k
- :base_type (value->base-type v)}))))
-
-
-(defn process-and-run [{:keys [native database] :as query}]
+(defn process-and-run
+ "Process and run a native (raw SQL) QUERY."
+ {:arglists '([query])}
+ [{{sql :query} :native
+ database-id :database :as query}]
+ {:pre [(string? sql)
+ (integer? database-id)]}
(log/debug "QUERY: " query)
- (let [db (sel :one Database :id database)
- sql (:query native)
- ;; this is where we actually run things
- ;; TODO - exception catching
- results (korma/exec-raw (korma-db db) sql :results)]
- {:status :completed
- :row_count (count results)
- :data {:rows (map vals results)
- :columns (keys (first results))
- :cols (get-cols (first results))}}))
+ (try (let [db (-> (sel :one Database :id database-id)
+ korma-db
+ korma.db/get-connection)
+ [columns & [first-row :as rows]] (jdbc/query db sql :as-arrays? true)]
+ {:status :completed
+ :row_count (count rows)
+ :data {:rows rows
+ :columns columns
+ :cols (map (fn [column first-value]
+ {:name column
+ :base_type (value->base-type first-value)})
+ columns first-row)}})
+ (catch java.sql.SQLException e
+ {:status :failed
+ :error (->> (.getMessage e) ; error message comes back like 'Column "ZID" not found; SQL statement: ... [error-code]
+ (re-find #"^(.*);") ; the user already knows the SQL, and error code is meaningless
+ second)}))) ; so just return the part of the exception that is relevant
diff --git a/test/metabase/driver/generic_sql/native_test.clj b/test/metabase/driver/generic_sql/native_test.clj
new file mode 100644
index 0000000000000000000000000000000000000000..f3de50e1425106907ff604a4cf8e59af7cd1e4b1
--- /dev/null
+++ b/test/metabase/driver/generic_sql/native_test.clj
@@ -0,0 +1,34 @@
+(ns metabase.driver.generic-sql.native-test
+ (:require [expectations :refer :all]
+ [metabase.driver.generic-sql.native :refer :all]
+ [metabase.test-data :refer :all]
+ metabase.test-setup))
+
+;; Just check that a basic query works
+(expect {:status :completed
+ :row_count 2
+ :data {:rows [[100]
+ [99]]
+ :columns [:id]
+ :cols [{:name :id, :base_type :IntegerField}]}}
+ (process-and-run {:native {:query "SELECT ID FROM VENUES ORDER BY ID DESC LIMIT 2;"}
+ :database @db-id}))
+
+;; Check that column ordering is maintained
+(expect
+ {:status :completed
+ :row_count 2
+ :data {:rows [[100 "Mohawk Bend" 46]
+ [99 "Golden Road Brewing" 10]]
+ :columns [:id :name :category_id]
+ :cols [{:name :id, :base_type :IntegerField}
+ {:name :name, :base_type :TextField}
+ {:name :category_id, :base_type :IntegerField}]}}
+ (process-and-run {:native {:query "SELECT ID, NAME, CATEGORY_ID FROM VENUES ORDER BY ID DESC LIMIT 2;"}
+ :database @db-id}))
+
+;; Check that we get proper error responses for malformed SQL
+(expect {:status :failed
+ :error "Column \"ZID\" not found"}
+ (process-and-run {:native {:query "SELECT ZID FROM CHECKINS LIMIT 2;"}
+ :database @db-id}))