Merge pull request #9731 from metabase/fix-memsql

Don't set sessionVariables=sql_mode='ALLOW_INVALID_DATES' for MySQL/MariaDB

src/metabase/driver/mysql.clj

@@ -280,10 +280,7 @@
    :characterEncoding    "UTF8"
    :characterSetResults  "UTF8"
    ;; GZIP compress packets sent between Metabase server and MySQL/MariaDB database
-   :useCompression       true
-   ;; allow inserting dates where value is '0000-00-00' -- this is disallowed by default on newer versions of MySQL,
-   ;; but we still want to test that we can handle it correctly for older ones
-   :sessionVariables     "sql_mode='ALLOW_INVALID_DATES'"})
+   :useCompression       true})
 
 (defmethod sql-jdbc.conn/connection-details->spec :mysql [_ {ssl? :ssl, :keys [additional-options], :as details}]
   ;; In versions older than 0.32.0 the MySQL driver did not correctly save `ssl?` connection status. Users worked

test/metabase/driver/mysql_test.clj

 (ns metabase.driver.mysql-test
   (:require [clj-time.core :as t]
+            [clojure.java.jdbc :as jdbc]
             [expectations :refer :all]
             [honeysql.core :as hsql]
             [metabase
@@ -16,7 +17,7 @@
              [util :as tu]]
             [metabase.test.data
              [datasets :refer [expect-with-driver]]
-             [interface :refer [def-database-definition]]]
+             [interface :as tx :refer [def-database-definition]]]
             [metabase.test.util.timezone :as tu.tz]
             [metabase.util.date :as du]
             [toucan.db :as db]
@@ -24,16 +25,30 @@
 
 ;; 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
-  [["exciting-moments-in-history"
-     [{:field-name "moment", :base-type :type/DateTime}]
-     [["0000-00-00"]]]])
-
 (expect-with-driver :mysql
   [[1 nil]]
-  (-> (data/dataset metabase.driver.mysql-test/all-zero-dates
-        (data/run-mbql-query exciting-moments-in-history))
-      qpt/rows))
+  (let [spec (sql-jdbc.conn/connection-details->spec :mysql (tx/dbdef->connection-details :mysql :server nil))]
+    (try
+      ;; Create the DB
+      (doseq [sql ["DROP DATABASE IF EXISTS all_zero_dates;"
+                   "CREATE DATABASE all_zero_dates;"]]
+        (jdbc/execute! spec [sql]))
+      ;; Create Table & add data
+      (let [details (tx/dbdef->connection-details :mysql :db {:database-name "all_zero_dates"})
+            spec    (-> (sql-jdbc.conn/connection-details->spec :mysql details)
+                        ;; allow inserting dates where value is '0000-00-00' -- this is disallowed by default on newer
+                        ;; versions of MySQL, but we still want to test that we can handle it correctly for older ones
+                        (assoc :sessionVariables "sql_mode='ALLOW_INVALID_DATES'"))]
+        (doseq [sql ["CREATE TABLE `exciting-moments-in-history` (`id` integer, `moment` timestamp);"
+                     "INSERT INTO `exciting-moments-in-history` (`id`, `moment`) VALUES (1, '0000-00-00');"]]
+          (jdbc/execute! spec [sql]))
+        ;; create & sync MB DB
+        (tt/with-temp Database [database {:engine "mysql", :details details}]
+          (sync/sync-database! database)
+          (data/with-db database
+            ;; run the query
+            (-> (data/run-mbql-query exciting-moments-in-history)
+                qpt/rows)))))))
 
 
 ;; Test how TINYINT(1) columns are interpreted. By default, they should be interpreted as integers, but with the
@@ -86,8 +101,8 @@
     (tu/db-timezone-id)))
 
 
-(def before-daylight-savings (du/str->date-time "2018-03-10 10:00:00" du/utc))
-(def after-daylight-savings (du/str->date-time "2018-03-12 10:00:00" du/utc))
+(def ^:private before-daylight-savings (du/str->date-time "2018-03-10 10:00:00" du/utc))
+(def ^:private after-daylight-savings  (du/str->date-time "2018-03-12 10:00:00" du/utc))
 
 (expect (#'mysql/timezone-id->offset-str "US/Pacific" before-daylight-savings) "-08:00")
 (expect (#'mysql/timezone-id->offset-str "US/Pacific" after-daylight-savings)  "-07:00")
@@ -156,10 +171,10 @@
       (qpt/first-row
         (du/with-effective-timezone (Database (data/id))
           (qp/process-query
-            {:database (data/id),
-             :type :native,
-             :settings {:report-timezone "UTC"}
-             :native     {:query "SELECT cast({{date}} as date)"
+            {:database   (data/id)
+             :type       :native
+             :settings   {:report-timezone "UTC"}
+             :native     {:query         "SELECT cast({{date}} as date)"
                           :template-tags {:date {:name "date" :display_name "Date" :type "date" }}}
              :parameters [{:type "date/single" :target ["variable" ["template-tag" "date"]] :value "2018-04-18"}]}))))))
 
@@ -173,7 +188,6 @@
    :classname            "org.mariadb.jdbc.Driver"
    :subprotocol          "mysql"
    :zeroDateTimeBehavior "convertToNull"
-   :sessionVariables     "sql_mode='ALLOW_INVALID_DATES'"
    :user                 "cam"
    :subname              "//localhost:3306/my_db"
    :useCompression       true