Allow custom MySQL JDBC connection string params

src/metabase/db/spec.clj

@@ -35,7 +35,7 @@
           :subprotocol "mysql"
           :subname (str "//" host ":" port "/" db)
           :delimiters "`"}
-         (dissoc opts :host :port :db)))
+         (dissoc opts :host :port :db :additional-options)))
 
 
 ;; TODO - These other ones can acutally be moved directly into their respective drivers themselves since they're not supported as backing DBs

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,28 @@
    :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
+  {:argslist '([connection-spec details])}
+  [{connection-string :subname, :as connection-spec} {ssl? :ssl, additional-options :additional-options, :as details}]
+  (assoc connection-spec
+    :subname (str connection-string
+                  "?"
+                  default-connection-args-string
+                  ;; newer versions of MySQL will complain if you don't explicitly disable SSL
+                  (when-not ssl?
+                    "&useSSL=false")
+                  (when (seq additional-options)
+                    (str "&" additional-options)))))
 
 (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)))
 
 (defn- unix-timestamp->timestamp [expr seconds-or-milliseconds]
   (hsql/call :from_unixtime (case seconds-or-milliseconds
@@ -161,7 +173,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

test/metabase/driver/mysql_test.clj

 (ns metabase.driver.mysql-test
   (:require [expectations :refer :all]
+            [metabase.db :as db]
+            (metabase.driver [generic-sql :as generic-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 (generic-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))))