From ecb42bbd777dc8f1d4bc187b3a9561613ccc721a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cam=20Sau=CC=88l?= <cameron@getluckybird.com>
Date: Mon, 29 Jun 2015 14:46:46 -0700
Subject: [PATCH] New driver method supports? to check if a driver supports a
feature
---
.dir-locals.el | 2 +-
src/metabase/driver/generic_sql.clj | 15 +++++++++-
src/metabase/driver/interface.clj | 30 +++++++++++++++++--
src/metabase/driver/mongo.clj | 8 +++++
src/metabase/driver/postgres.clj | 3 +-
src/metabase/driver/query_processor.clj | 2 +-
.../driver/query_processor/expand.clj | 22 ++++++++++----
test/metabase/driver/query_processor_test.clj | 21 +++++++++++++
8 files changed, 90 insertions(+), 13 deletions(-)
diff --git a/.dir-locals.el b/.dir-locals.el
index 3120b81a99d..22c74497449 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -21,7 +21,7 @@
(expect-eval-actual-first 1)
(expect-expansion 0)
(expect-let 1)
- (expect-when-testing-against-dataset 1)
+ (expect-when-testing-dataset 1)
(expect-when-testing-mongo 1)
(expect-with-all-drivers 1)
(expect-with-dataset 1)
diff --git a/src/metabase/driver/generic_sql.clj b/src/metabase/driver/generic_sql.clj
index a08274d9a00..505686f5724 100644
--- a/src/metabase/driver/generic_sql.clj
+++ b/src/metabase/driver/generic_sql.clj
@@ -8,7 +8,15 @@
(metabase.driver.generic-sql [query-processor :as qp]
[util :refer :all])))
-(defrecord SqlDriver [column->base-type
+(def ^:private ^:const sql-driver-features
+ "Features supported by *all* Generic SQL drivers."
+ #{:foreign-keys
+ :standard-deviation-aggregations
+ :unix-timestamp-special-type-fields})
+
+(defrecord SqlDriver [;; A set of additional features supported by a specific driver implmentation, e.g. :set-timezone for :postgres
+ additional-supported-features
+ column->base-type
connection-details->connection-spec
database->connection-details
sql-string-length-fn
@@ -20,6 +28,11 @@
;; e.g. #"CAST\(TIMESTAMPADD\('(?:MILLI)?SECOND', ([^\s]+), DATE '1970-01-01'\) AS DATE\)" for H2
uncastify-timestamp-regex]
IDriver
+ ;; Features
+ (supports? [_ feature]
+ (or (contains? sql-driver-features feature)
+ (contains? additional-supported-features feature)))
+
;; Connection
(can-connect? [this database]
(can-connect-with-details? this (database->connection-details database)))
diff --git a/src/metabase/driver/interface.clj b/src/metabase/driver/interface.clj
index cdd370f6ee4..d13bef75b43 100644
--- a/src/metabase/driver/interface.clj
+++ b/src/metabase/driver/interface.clj
@@ -1,21 +1,36 @@
(ns metabase.driver.interface
- "Protocols that DB drivers implement. Thus, the interface such drivers provide.")
+ "Protocols that DB drivers implement. Thus, the interface such drivers provide."
+ (:import (clojure.lang Keyword)))
+
+(def ^:const driver-optional-features
+ "A set on optional features (as keywords) that may or may not be supported by individual drivers."
+ #{:foreign-keys
+ :set-timezone
+ :standard-deviation-aggregations
+ :unix-timestamp-special-type-fields})
+
;; ## IDriver Protocol
(defprotocol IDriver
"Methods all drivers must implement."
+ ;; Features
+ (supports? [this ^Keyword feature]
+ "Does this driver support optional FEATURE?
+
+ (supports? metabase.driver.h2/driver :timestamps) -> false")
+
;; Connection
(can-connect? [this database]
"Check whether we can connect to DATABASE and perform a simple query.
(To check whether we can connect to a database given only its details, use `can-connect-with-details?` instead).
- (can-connect? (sel :one Database :id 1))")
+ (can-connect? driver (sel :one Database :id 1))")
(can-connect-with-details? [this details-map]
"Check whether we can connect to a database and performa a simple query.
Returns true if we can, otherwise returns false or throws an Exception.
- (can-connect-with-details? {:engine :postgres, :dbname \"book\", ...})")
+ (can-connect-with-details? driver {:engine :postgres, :dbname \"book\", ...})")
;; Syncing
(sync-in-context [this database do-sync-fn]
@@ -80,3 +95,12 @@
If a driver doesn't implement this protocol, it *must* implement `ISyncDriverFieldValues`."
(field-percent-urls [this field]
"Return the percentage of non-nil values of textual FIELD that are valid URLs."))
+
+
+;; ## Helper Functions
+
+(defn assert-driver-supports
+ "Helper fn. Assert that DRIVER supports FEATURE."
+ [driver ^Keyword feature]
+ (when-not (supports? driver feature)
+ (throw (Exception. (format "%s is not supported by this driver." (name feature))))))
diff --git a/src/metabase/driver/mongo.clj b/src/metabase/driver/mongo.clj
index 20358519d30..37f3b1d0446 100644
--- a/src/metabase/driver/mongo.clj
+++ b/src/metabase/driver/mongo.clj
@@ -40,8 +40,16 @@
;;; ## MongoDriver
+(def ^:const ^:private mongo-driver-features
+ "Optional features supported by the Mongo driver."
+ #{}) ; nothing yet
+
(deftype MongoDriver []
IDriver
+;;; ### Features
+ (supports? [_ feature]
+ (contains? mongo-driver-features feature))
+
;;; ### Connection
(can-connect? [_ database]
(with-mongo-connection [^com.mongodb.DBApiLayer conn database]
diff --git a/src/metabase/driver/postgres.clj b/src/metabase/driver/postgres.clj
index df7332e4dcf..18a8763a077 100644
--- a/src/metabase/driver/postgres.clj
+++ b/src/metabase/driver/postgres.clj
@@ -125,7 +125,8 @@
(def ^:const driver
(generic-sql/map->SqlDriver
- {:column->base-type column->base-type
+ {:additional-supported-features #{:set-timezone}
+ :column->base-type column->base-type
:connection-details->connection-spec connection-details->connection-spec
:database->connection-details database->connection-details
:sql-string-length-fn :CHAR_LENGTH
diff --git a/src/metabase/driver/query_processor.clj b/src/metabase/driver/query_processor.clj
index 244fe2a71a7..d720d20230d 100644
--- a/src/metabase/driver/query_processor.clj
+++ b/src/metabase/driver/query_processor.clj
@@ -50,7 +50,7 @@
(defn- pre-expand [qp]
(fn [query]
- (qp (expand/expand query))))
+ (qp (expand/expand *driver* query))))
(defn- post-add-row-count-and-status
diff --git a/src/metabase/driver/query_processor/expand.clj b/src/metabase/driver/query_processor/expand.clj
index 78f52a93ade..4d3ec5b8fdc 100644
--- a/src/metabase/driver/query_processor/expand.clj
+++ b/src/metabase/driver/query_processor/expand.clj
@@ -42,6 +42,7 @@
[medley.core :as m]
[swiss.arrows :refer [-<>]]
[metabase.db :refer [sel]]
+ [metabase.driver.interface :as i]
(metabase.models [database :refer [Database]]
[field :as field]
[foreign-key :refer [ForeignKey]]
@@ -80,6 +81,12 @@
;; ## -------------------- Expansion - Impl --------------------
+(def ^:private ^:dynamic *driver* nil)
+
+(defn- assert-driver-supports [^Keyword feature]
+ {:pre [*driver*]}
+ (i/assert-driver-supports *driver* feature))
+
(defn- non-empty-clause? [clause]
(and clause
(or (not (sequential? clause))
@@ -185,8 +192,9 @@
(defn expand
"Expand a QUERY-DICT."
- [query-dict]
- (binding [*field-ids* (atom #{})
+ [driver query-dict]
+ (binding [*driver* driver
+ *field-ids* (atom #{})
*fk-field-ids* (atom #{})
*table-ids* (atom #{})]
(some-> query-dict
@@ -272,9 +280,10 @@
(->FieldPlaceholder field-id))
["fk->"
(fk-field-id :guard integer?)
- (dest-field-id :guard integer?)] (do (swap! *field-ids* conj dest-field-id)
- (swap! *fk-field-ids* conj fk-field-id)
- (->FieldPlaceholder dest-field-id))))
+ (dest-field-id :guard integer?)] (do (assert-driver-supports :foreign-keys)
+ (swap! *field-ids* conj dest-field-id)
+ (swap! *fk-field-ids* conj fk-field-id)
+ (->FieldPlaceholder dest-field-id))))
([field-id value]
(->ValuePlaceholder (:field-id (ph field-id)) value)))
@@ -300,7 +309,8 @@
["avg" field-id] (->Aggregation :avg (ph field-id))
["count" field-id] (->Aggregation :count (ph field-id))
["distinct" field-id] (->Aggregation :distinct (ph field-id))
- ["stddev" field-id] (->Aggregation :stddev (ph field-id))
+ ["stddev" field-id] (do (assert-driver-supports :standard-deviation-aggregations)
+ (->Aggregation :stddev (ph field-id)))
["sum" field-id] (->Aggregation :sum (ph field-id))
["cum_sum" field-id] (->Aggregation :cumulative-sum (ph field-id)))
diff --git a/test/metabase/driver/query_processor_test.clj b/test/metabase/driver/query_processor_test.clj
index 192bfcf1482..704fc114001 100644
--- a/test/metabase/driver/query_processor_test.clj
+++ b/test/metabase/driver/query_processor_test.clj
@@ -622,6 +622,15 @@
{:source_table (id :venues)
:aggregation ["stddev" (id :venues :latitude)]})
+;; Make sure standard deviation fails for the Mongo driver since its not supported
+(datasets/expect-with-dataset :mongo
+ {:status :failed
+ :error "standard-deviation-aggregations is not supported by this driver."}
+ (driver/process-query {:database (db-id)
+ :type :query
+ :query {:source_table (id :venues)
+ :aggregation ["stddev" (id :venues :latitude)]}}))
+
;;; ## order_by aggregate fields (SQL-only for the time being)
@@ -891,3 +900,15 @@
[&sightings.id:id "ascending"]]
:limit 10)
:data :rows (map butlast) (map reverse))) ; drop timestamps. reverse ordering to make the results columns order match order_by
+
+
+;; Check that trying to use a Foreign Key fails for Mongo
+(datasets/expect-with-dataset :mongo
+ {:status :failed
+ :error "foreign-keys is not supported by this driver."}
+ (query-with-temp-db defs/tupac-sightings
+ :source_table &sightings:id
+ :order_by [[["fk->" &sightings.city_id:id &cities.name:id] "ascending"]
+ [["fk->" &sightings.category_id:id &categories.name:id] "descending"]
+ [&sightings.id:id "ascending"]]
+ :limit 10))
--
GitLab