diff --git a/src/metabase/driver/generic_sql.clj b/src/metabase/driver/generic_sql.clj
index 2559cbcfccb554734be9c3692a5bd3812248ff49..3a579746113c345c852b8188ce99c12fe1390217 100644
--- a/src/metabase/driver/generic_sql.clj
+++ b/src/metabase/driver/generic_sql.clj
@@ -1,7 +1,8 @@
(ns metabase.driver.generic-sql
(:require [clojure.java.jdbc :as jdbc]
[clojure.math.numeric-tower :as math]
- [clojure.set :as set]
+ (clojure [set :as set]
+ [string :as str])
[clojure.tools.logging :as log]
(honeysql [core :as hsql]
[format :as hformat])
@@ -151,6 +152,16 @@
[{:keys [engine details], :as database}]
(db->pooled-connection-spec database))
+(defn handle-additional-options
+ "If DETAILS contains an `:addtional-options` key, append those options to the connection string in CONNECTION-SPEC.
+ (Some drivers like MySQL provide this details field to allow special behavior where needed)."
+ {:arglists '([connection-spec details])}
+ [{connection-string :subname, :as connection-spec} {additional-options :additional-options, :as details}]
+ (-> (dissoc connection-spec :additional-options)
+ (assoc :subname (str connection-string (when (seq additional-options)
+ (str (if (str/includes? connection-string "?") "&" "?")
+ additional-options))))))
+
(defn escape-field-name
"Escape dots in a field name so HoneySQL doesn't get confused and separate them. Returns a keyword."
diff --git a/src/metabase/driver/mysql.clj b/src/metabase/driver/mysql.clj
index 9b7745cd575395ae6d6508cf5c7986f1488dedd7..100576a657fd9963ad0fc7b354e0dcca4b542e92 100644
--- a/src/metabase/driver/mysql.clj
+++ b/src/metabase/driver/mysql.clj
@@ -43,7 +43,7 @@
:VARCHAR :type/Text
:YEAR :type/Integer} (keyword (s/replace (name column-type) #"\sUNSIGNED$" "")))) ; strip off " UNSIGNED" from end if present
-(def ^:private ^:const connection-args
+(def ^:private ^:const default-connection-args
"Map of args for the MySQL JDBC connection string.
Full list of is options is available here: http://dev.mysql.com/doc/connector-j/6.0/en/connector-j-reference-configuration-properties.html"
{:zeroDateTimeBehavior :convertToNull ; 0000-00-00 dates are valid in MySQL; convert these to `null` when they come back because they're illegal in Java
@@ -51,16 +51,25 @@
:characterEncoding :UTF8
:characterSetResults :UTF8})
-(def ^:private ^:const connection-args-string
- (apply str "?" (interpose \& (for [[k v] connection-args]
- (str (name k) \= (name v))))))
+(def ^:private ^:const ^String default-connection-args-string
+ (s/join \& (for [[k v] default-connection-args]
+ (str (name k) \= (name v)))))
+
+(defn- append-connection-args
+ "Append `default-connection-args-string` to the connection string in CONNECTION-DETAILS, and an additional option to explicitly disable SSL if appropriate.
+ (Newer versions of MySQL will complain if you don't explicitly disable SSL.)"
+ {:argslist '([connection-spec details])}
+ [{connection-string :subname, :as connection-spec} {ssl? :ssl}]
+ (assoc connection-spec
+ :subname (str connection-string "?" default-connection-args-string (when-not ssl?
+ "&useSSL=false"))))
(defn- connection-details->spec [details]
(-> details
(set/rename-keys {:dbname :db})
dbspec/mysql
- (update :subname #(str % connection-args-string (when-not (:ssl details)
- "&useSSL=false"))))) ; newer versions of MySQL will complain if you don't explicitly disable SSL
+ (append-connection-args details)
+ (sql/handle-additional-options details)))
(defn- unix-timestamp->timestamp [expr seconds-or-milliseconds]
(hsql/call :from_unixtime (case seconds-or-milliseconds
@@ -161,7 +170,10 @@
{:name "password"
:display-name "Database password"
:type :password
- :placeholder "*******"}])
+ :placeholder "*******"}
+ {:name "additional-options"
+ :display-name "Additional JDBC connection string options"
+ :placeholder "tinyInt1isBit=false"}])
:humanize-connection-error-message (u/drop-first-arg humanize-connection-error-message)})
sql/ISQLDriver
diff --git a/src/metabase/driver/postgres.clj b/src/metabase/driver/postgres.clj
index 0a015841f1af3f66690709f340a7253d9211b276..294ec44ee3c3119aa429fcbd8e22c2a0455cffcb 100644
--- a/src/metabase/driver/postgres.clj
+++ b/src/metabase/driver/postgres.clj
@@ -15,67 +15,66 @@
(:import java.util.UUID
org.postgresql.ssl.NonValidatingFactory))
-(defn- column->base-type
+(def ^:private ^:const column->base-type
"Map of Postgres column types -> Field base types.
Add more mappings here as you come across them."
- [column-type]
- ({:bigint :type/BigInteger
- :bigserial :type/BigInteger
- :bit :type/*
- :bool :type/Boolean
- :boolean :type/Boolean
- :box :type/*
- :bpchar :type/Text ; "blank-padded char" is the internal name of "character"
- :bytea :type/* ; byte array
- :cidr :type/Text ; IPv4/IPv6 network address
- :circle :type/*
- :date :type/Date
- :decimal :type/Decimal
- :float4 :type/Float
- :float8 :type/Float
- :geometry :type/*
- :inet :type/IPAddress
- :int :type/Integer
- :int2 :type/Integer
- :int4 :type/Integer
- :int8 :type/BigInteger
- :interval :type/* ; time span
- :json :type/Text
- :jsonb :type/Text
- :line :type/*
- :lseg :type/*
- :macaddr :type/Text
- :money :type/Decimal
- :numeric :type/Decimal
- :path :type/*
- :pg_lsn :type/Integer ; PG Log Sequence #
- :point :type/*
- :real :type/Float
- :serial :type/Integer
- :serial2 :type/Integer
- :serial4 :type/Integer
- :serial8 :type/BigInteger
- :smallint :type/Integer
- :smallserial :type/Integer
- :text :type/Text
- :time :type/Time
- :timetz :type/Time
- :timestamp :type/DateTime
- :timestamptz :type/DateTime
- :tsquery :type/*
- :tsvector :type/*
- :txid_snapshot :type/*
- :uuid :type/UUID
- :varbit :type/*
- :varchar :type/Text
- :xml :type/Text
- (keyword "bit varying") :type/*
- (keyword "character varying") :type/Text
- (keyword "double precision") :type/Float
- (keyword "time with time zone") :type/Time
- (keyword "time without time zone") :type/Time
- (keyword "timestamp with timezone") :type/DateTime
- (keyword "timestamp without timezone") :type/DateTime} column-type))
+ {:bigint :type/BigInteger
+ :bigserial :type/BigInteger
+ :bit :type/*
+ :bool :type/Boolean
+ :boolean :type/Boolean
+ :box :type/*
+ :bpchar :type/Text ; "blank-padded char" is the internal name of "character"
+ :bytea :type/* ; byte array
+ :cidr :type/Text ; IPv4/IPv6 network address
+ :circle :type/*
+ :date :type/Date
+ :decimal :type/Decimal
+ :float4 :type/Float
+ :float8 :type/Float
+ :geometry :type/*
+ :inet :type/IPAddress
+ :int :type/Integer
+ :int2 :type/Integer
+ :int4 :type/Integer
+ :int8 :type/BigInteger
+ :interval :type/* ; time span
+ :json :type/Text
+ :jsonb :type/Text
+ :line :type/*
+ :lseg :type/*
+ :macaddr :type/Text
+ :money :type/Decimal
+ :numeric :type/Decimal
+ :path :type/*
+ :pg_lsn :type/Integer ; PG Log Sequence #
+ :point :type/*
+ :real :type/Float
+ :serial :type/Integer
+ :serial2 :type/Integer
+ :serial4 :type/Integer
+ :serial8 :type/BigInteger
+ :smallint :type/Integer
+ :smallserial :type/Integer
+ :text :type/Text
+ :time :type/Time
+ :timetz :type/Time
+ :timestamp :type/DateTime
+ :timestamptz :type/DateTime
+ :tsquery :type/*
+ :tsvector :type/*
+ :txid_snapshot :type/*
+ :uuid :type/UUID
+ :varbit :type/*
+ :varchar :type/Text
+ :xml :type/Text
+ (keyword "bit varying") :type/*
+ (keyword "character varying") :type/Text
+ (keyword "double precision") :type/Float
+ (keyword "time with time zone") :type/Time
+ (keyword "time without time zone") :type/Time
+ (keyword "timestamp with timezone") :type/DateTime
+ (keyword "timestamp without timezone") :type/DateTime})
(defn- column->special-type
"Attempt to determine the special-type of a Field given its name and Postgres column type."
@@ -96,17 +95,21 @@
"Params to include in the JDBC connection spec to disable SSL."
{:sslmode "disable"})
-(defn- connection-details->spec [{:keys [ssl] :as details-map}]
+(defn- connection-details->spec [{ssl? :ssl, :as details-map}]
(-> details-map
(update :port (fn [port]
- (if (string? port) (Integer/parseInt port)
- port)))
- (dissoc :ssl) ; remove :ssl in case it's false; DB will still try (& fail) to connect if the key is there
- (merge (if ssl
+ (if (string? port)
+ (Integer/parseInt port)
+ port)))
+ ;; remove :ssl in case it's false; DB will still try (& fail) to connect if the key is there
+ (dissoc :ssl)
+ (merge (if ssl?
ssl-params
disable-ssl-params))
(rename-keys {:dbname :db})
- dbspec/postgres))
+ dbspec/postgres
+ (sql/handle-additional-options details-map)))
+
(defn- unix-timestamp->timestamp [expr seconds-or-milliseconds]
(case seconds-or-milliseconds
@@ -241,7 +244,10 @@
{:name "ssl"
:display-name "Use a secure connection (SSL)?"
:type :boolean
- :default false}])
+ :default false}
+ {:name "additional-options"
+ :display-name "Additional JDBC connection string options"
+ :placeholder "prepareThreshold=0"}])
:humanize-connection-error-message (u/drop-first-arg humanize-connection-error-message)})
sql/ISQLDriver PostgresISQLDriverMixin)
diff --git a/test/metabase/driver/mysql_test.clj b/test/metabase/driver/mysql_test.clj
index a4376b99c8599e9dca5d6bae85bf903ddd9a4df5..f30f72fd23a431da35edf3dc11100413d958f9a5 100644
--- a/test/metabase/driver/mysql_test.clj
+++ b/test/metabase/driver/mysql_test.clj
@@ -1,8 +1,16 @@
(ns metabase.driver.mysql-test
(:require [expectations :refer :all]
+ [metabase.db :as db]
+ (metabase.driver [generic-sql :as sql]
+ mysql)
+ [metabase.models.database :refer [Database]]
+ [metabase.sync-database :as sync-db]
[metabase.test.data :as data]
(metabase.test.data [datasets :refer [expect-with-engine]]
- [interface :refer [def-database-definition]])))
+ [interface :refer [def-database-definition]])
+ [metabase.test.util :as tu]
+ [metabase.util :as u])
+ (:import metabase.driver.mysql.MySQLDriver))
;; MySQL allows 0000-00-00 dates, but JDBC does not; make sure that MySQL is converting them to NULL when returning them like we asked
(def-database-definition ^:private ^:const all-zero-dates
@@ -12,6 +20,51 @@
(expect-with-engine :mysql
[[1 nil]]
+ ;; TODO - use the `rows` function from `metabse.query-processor-test`. Preferrably after it's moved to some sort of shared test util namespace
(-> (data/dataset metabase.driver.mysql-test/all-zero-dates
(data/run-query exciting-moments-in-history))
:data :rows))
+
+
+;; make sure connection details w/ extra params work as expected
+(expect
+ "//localhost:3306/cool?zeroDateTimeBehavior=convertToNull&useUnicode=true&characterEncoding=UTF8&characterSetResults=UTF8&useSSL=false&tinyInt1isBit=false"
+ (:subname (sql/connection-details->spec (MySQLDriver.) {:host "localhost"
+ :port "3306"
+ :dbname "cool"
+ :additional-options "tinyInt1isBit=false"})))
+
+
+;; Test how TINYINT(1) columns are interpreted. By default, they should be interpreted as integers,
+;; but with the correct additional options, we should be able to change that -- see https://github.com/metabase/metabase/issues/3506
+(def-database-definition ^:private ^:const tiny-int-ones
+ ["number-of-cans"
+ [{:field-name "thing", :base-type :type/Text}
+ {:field-name "number-of-cans", :base-type {:native "tinyint(1)"}}]
+ [["Six Pack" 6]
+ ["Toucan" 2]
+ ["Empty Vending Machine" 0]]])
+
+(defn- db->fields [db]
+ (let [table-ids (db/select-ids 'Table :db_id (u/get-id db))]
+ (set (map (partial into {}) (db/select ['Field :name :base_type :special_type] :table_id [:in table-ids])))))
+
+;; By default TINYINT(1) should be a boolean
+(expect-with-engine :mysql
+ #{{:name "number-of-cans", :base_type :type/Boolean, :special_type :type/Category}
+ {:name "id", :base_type :type/Integer, :special_type :type/PK}
+ {:name "thing", :base_type :type/Text, :special_type :type/Category}}
+ (data/with-temp-db [db tiny-int-ones]
+ (db->fields db)))
+
+;; if someone says specifies `tinyInt1isBit=false`, it should come back as a number instead
+(expect-with-engine :mysql
+ #{{:name "number-of-cans", :base_type :type/Integer, :special_type :type/Category}
+ {:name "id", :base_type :type/Integer, :special_type :type/PK}
+ {:name "thing", :base_type :type/Text, :special_type :type/Category}}
+ (data/with-temp-db [db tiny-int-ones]
+ (tu/with-temp Database [db {:engine "mysql"
+ :details (assoc (:details db)
+ :additional-options "tinyInt1isBit=false")}]
+ (sync-db/sync-database! db)
+ (db->fields db))))
diff --git a/test/metabase/driver/postgres_test.clj b/test/metabase/driver/postgres_test.clj
index ed69b4550c31a5572e4150e11d5ed0da0425f3cf..10d67ae97168886520468d50192b9423a5879405 100644
--- a/test/metabase/driver/postgres_test.clj
+++ b/test/metabase/driver/postgres_test.clj
@@ -16,7 +16,7 @@
[metabase.util :as u])
(:import metabase.driver.postgres.PostgresDriver))
-(def ^:private pg-driver (PostgresDriver.))
+(def ^:private ^PostgresDriver pg-driver (PostgresDriver.))
;; # Check that SSL params get added the connection details in the way we'd like
;; ## no SSL -- this should *not* include the key :ssl (regardless of its value) since that will cause the PG driver to use SSL anyway
@@ -213,3 +213,12 @@
(expect-with-engine :postgres
(get-timezone-with-report-timezone nil)
(get-timezone-with-report-timezone "Crunk Burger"))
+
+
+;; make sure connection details w/ extra params work as expected
+(expect
+ "//localhost:5432/cool?prepareThreshold=0"
+ (:subname (sql/connection-details->spec pg-driver {:host "localhost"
+ :port "5432"
+ :dbname "cool"
+ :additional-options "prepareThreshold=0"})))