Skip to content
Snippets Groups Projects
Commit 58581e3e authored by Cam Saül's avatar Cam Saül
Browse files

Fix missing type information for mongo native queries :keyboard:

parent 3e5ec1f1
Branches
Tags
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/driver.clj
+ 2
− 1
View file @ 58581e3e
......@@ -286,7 +286,8 @@
(contains? (features driver) feature))
(defn class->base-type
"Return the `Field.base_type` that corresponds to a given class returned by the DB."
"Return the `Field.base_type` that corresponds to a given class returned by the DB.
This is used to infer the types of results that come back from native queries."
[klass]
(or ({Boolean :BooleanField
Double :FloatField
......
src/metabase/driver/generic_sql.clj
+ 29
− 25
View file @ 58581e3e
......@@ -102,11 +102,13 @@
"Return a korma form appropriate for converting a Unix timestamp integer field or value to an proper SQL `Timestamp`.
SECONDS-OR-MILLISECONDS refers to the resolution of the int in question and with be either `:seconds` or `:milliseconds`."))
;; This does something important for the Crate driver, apparently (what?)
(extend-protocol jdbc/IResultSetReadColumn
(class (object-array []))
(result-set-read-column [x _ _] (PersistentVector/adopt x)))
(def ^:dynamic ^:private connection-pools
"A map of our currently open connection pools, keyed by DATABASE `:id`."
(atom {}))
......@@ -254,6 +256,7 @@
(float (/ url-count total-count))
0.0))
;; TODO - Full table scan!?! Maybe just fetch first N non-nil values and do in Clojure-land instead
(defn slow-field-percent-urls
"Slow implementation of `field-percent-urls` that (probably) requires a full table scan.
Only use this for DBs where `fast-field-percent-urls` doesn't work correctly, like SQLServer."
......@@ -330,11 +333,11 @@
[^DatabaseMetaData metadata, driver, {:keys [schema name]}]
(set (for [{:keys [column_name type_name]} (jdbc/result-set-seq (.getColumns metadata nil schema name nil))
:let [calculated-special-type (column->special-type driver column_name (keyword type_name))]]
(merge {:name column_name
:custom {:column-type type_name}
:base-type (or (column->base-type driver (keyword type_name))
(do (log/warn (format "Don't know how to map column type '%s' to a Field base_type, falling back to :UnknownField." type_name))
:UnknownField))}
(merge {:name column_name
:custom {:column-type type_name}
:base-type (or (column->base-type driver (keyword type_name))
(do (log/warn (format "Don't know how to map column type '%s' to a Field base_type, falling back to :UnknownField." type_name))
:UnknownField))}
(when calculated-special-type
{:special-type calculated-special-type})))))
......@@ -346,8 +349,9 @@
set)]
(update table :fields (fn [fields]
(set (for [field fields]
(if-not (contains? pks (:name field)) field
(assoc field :pk? true))))))))
(if-not (contains? pks (:name field))
field
(assoc field :pk? true))))))))
(defn describe-database
"Default implementation of `describe-database` for JDBC-based drivers."
......@@ -387,24 +391,24 @@
"Default implementations for methods in `ISQLDriver`."
[]
(require 'metabase.driver.generic-sql.query-processor)
{:active-tables fast-active-tables
:apply-aggregation (resolve 'metabase.driver.generic-sql.query-processor/apply-aggregation) ; don't resolve the vars yet so during interactive dev if the
:apply-breakout (resolve 'metabase.driver.generic-sql.query-processor/apply-breakout) ; underlying impl changes we won't have to reload all the drivers
:apply-fields (resolve 'metabase.driver.generic-sql.query-processor/apply-fields)
:apply-filter (resolve 'metabase.driver.generic-sql.query-processor/apply-filter)
:apply-join-tables (resolve 'metabase.driver.generic-sql.query-processor/apply-join-tables)
:apply-limit (resolve 'metabase.driver.generic-sql.query-processor/apply-limit)
:apply-order-by (resolve 'metabase.driver.generic-sql.query-processor/apply-order-by)
:apply-page (resolve 'metabase.driver.generic-sql.query-processor/apply-page)
:column->special-type (constantly nil)
:current-datetime-fn (constantly :%now)
:excluded-schemas (constantly nil)
:field->alias (u/drop-first-arg name)
:field-percent-urls fast-field-percent-urls
:prepare-value (u/drop-first-arg :value)
:quote-style (constantly :ansi)
:set-timezone-sql (constantly nil)
:stddev-fn (constantly :STDDEV)})
{:active-tables fast-active-tables
:apply-aggregation (resolve 'metabase.driver.generic-sql.query-processor/apply-aggregation) ; don't resolve the vars yet so during interactive dev if the
:apply-breakout (resolve 'metabase.driver.generic-sql.query-processor/apply-breakout) ; underlying impl changes we won't have to reload all the drivers
:apply-fields (resolve 'metabase.driver.generic-sql.query-processor/apply-fields)
:apply-filter (resolve 'metabase.driver.generic-sql.query-processor/apply-filter)
:apply-join-tables (resolve 'metabase.driver.generic-sql.query-processor/apply-join-tables)
:apply-limit (resolve 'metabase.driver.generic-sql.query-processor/apply-limit)
:apply-order-by (resolve 'metabase.driver.generic-sql.query-processor/apply-order-by)
:apply-page (resolve 'metabase.driver.generic-sql.query-processor/apply-page)
:column->special-type (constantly nil)
:current-datetime-fn (constantly :%now)
:excluded-schemas (constantly nil)
:field->alias (u/drop-first-arg name)
:field-percent-urls fast-field-percent-urls
:prepare-value (u/drop-first-arg :value)
:quote-style (constantly :ansi)
:set-timezone-sql (constantly nil)
:stddev-fn (constantly :STDDEV)})
(defn IDriverSQLDefaultsMixin
......
src/metabase/driver/mongo/query_processor.clj
+ 1
− 1
View file @ 58581e3e
......@@ -413,4 +413,4 @@
{:columns columns
:rows (for [row results]
(mapv row columns))
:annotate? true}))
:annotate? mbql?}))
src/metabase/query_processor.clj
+ 27
− 19
View file @ 58581e3e
......@@ -168,7 +168,7 @@
(qp query))))
(defn- pre-expand-resolve
"Transforms an MBQL into an expanded form with more information and structure. Also resolves references to fields, tables,
"Transforms an MBQL into an expanded form with more information and structure. Also resolves references to fields, tables,
etc, into their concrete details which are necessary for query formation by the executing driver."
[qp]
(fn [{database-id :database, :as query}]
......@@ -198,14 +198,15 @@
(defn- format-rows [{:keys [report-timezone]} rows]
(for [row rows]
(for [v row]
(if (u/is-temporal? v)
;; NOTE: if we don't have an explicit report-timezone then use the JVM timezone
;; this ensures alignment between the way dates are processed by JDBC and our returned data
;; GH issues: #2282, #2035
(u/->iso-8601-datetime v (or report-timezone (System/getProperty "user.timezone")))
v))))
(let [timezone (or report-timezone (System/getProperty "user.timezone"))]
(for [row rows]
(for [v row]
(if (u/is-temporal? v)
;; NOTE: if we don't have an explicit report-timezone then use the JVM timezone
;; this ensures alignment between the way dates are processed by JDBC and our returned data
;; GH issues: #2282, #2035
(u/->iso-8601-datetime v timezone)
v)))))
(defn- post-format-rows
"Format individual query result values as needed. Ex: format temporal values as iso8601 strings w/ timezone."
......@@ -454,6 +455,16 @@
^String [{{:keys [executed-by uuid query-hash query-type], :as info} :info}]
(format "Metabase:: userID: %s executionID: %s queryType: %s queryHash: %s" executed-by uuid query-type query-hash))
(defn- infer-column-types
"Infer the types of columns by looking at the first value for each in the results, and add the relevant information in `:cols`.
This is used for native queries, which don't have the type information from the original `Field` objects used in the query, which is added to the results by `annotate`."
[results]
(assoc results
:columns (mapv name (:columns results))
:cols (vec (for [[column first-value] (partition 2 (interleave (:columns results) (first (:rows results))))]
{:name (name column)
:base_type (driver/class->base-type (type first-value))}))))
(defn- run-query
"The end of the QP middleware which actually executes the query on the driver.
......@@ -473,10 +484,7 @@
raw-result (driver/execute-query (:driver query) native-query)
query-result (if-not (or (mbql-query? query)
(:annotate? raw-result))
(assoc raw-result :columns (mapv name (:columns raw-result))
:cols (for [[column first-value] (partition 2 (interleave (:columns raw-result) (first (:rows raw-result))))]
{:name (name column)
:base_type (driver/class->base-type (type first-value))}))
(infer-column-types raw-result)
(annotate/annotate query raw-result))]
(assoc query-result :native_form native-form)))
......@@ -543,7 +551,7 @@
;;; | DATASET-QUERY PUBLIC API |
;;; +----------------------------------------------------------------------------------------------------+
(declare query-fail query-complete save-query-execution)
(declare query-fail query-complete save-query-execution!)
(defn- assert-valid-query-result [query-result]
(when-not (contains? query-result :status)
......@@ -600,7 +608,7 @@
(log/error (u/format-color 'red "Query failure: %s\n%s" (.getMessage e) (u/pprint-to-str (u/filtered-stacktrace e))))
(query-fail query-execution (.getMessage e))))))
(defn query-fail
(defn- query-fail
"Save QueryExecution state and construct a failed query response"
[query-execution error-message]
(let [updates {:status :failed
......@@ -611,7 +619,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
......@@ -620,7 +628,7 @@
:cols []
:columns []}))))
(defn query-complete
(defn- query-complete
"Save QueryExecution state and construct a completed (successful) query response"
[query-execution query-result]
;; record our query execution and format response
......@@ -631,12 +639,12 @@
(: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)))
(defn save-query-execution
(defn- save-query-execution!
"Save (or update) a `QueryExecution`."
[{:keys [id], :as query-execution}]
(if id
......
test/metabase/driver/mongo_test.clj
+ 12
− 12
View file @ 58581e3e
......@@ -14,7 +14,7 @@
;; ## Logic for selectively running mongo
(defmacro expect-when-testing-mongo [expected actual]
(defmacro expect-when-testing-mongo {:style/indent 0} [expected actual]
`(datasets/expect-when-testing-engine :mongo
~expected
~actual))
......@@ -75,13 +75,13 @@
{\"$project\": {\"_id\": false, \"count\": true}}]")
(expect-when-testing-mongo
{:status :completed
{:status :completed
:row_count 1
:data {:rows [[1]]
:columns ["count"]
:cols [{:description nil, :table_id nil, :special_type nil, :name "count", :extra_info {}, :id nil, :target nil, :display_name "count", :preview-display true, :base_type :UnknownField}]
:native_form {:collection "venues"
:query native-query}}}
:data {:rows [[1]]
:columns ["count"]
:cols [{:name "count", :base_type :IntegerField}]
:native_form {:collection "venues"
:query native-query}}}
(qp/process-query {:native {:query native-query
:collection "venues"}
:type :native
......@@ -91,11 +91,11 @@
;; DESCRIBE-DATABASE
(expect-when-testing-mongo
{:tables #{{:schema nil, :name "checkins"}
{:schema nil, :name "categories"}
{:schema nil, :name "users"}
{:schema nil, :name "venues"}}}
(driver/describe-database (MongoDriver.) (mongo-db)))
{:tables #{{:schema nil, :name "checkins"}
{:schema nil, :name "categories"}
{:schema nil, :name "users"}
{:schema nil, :name "venues"}}}
(driver/describe-database (MongoDriver.) (mongo-db)))
;; DESCRIBE-TABLE
(expect-when-testing-mongo
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment