From a7f9eecebdfaf91dfa22e3116da388d79cd8863a Mon Sep 17 00:00:00 2001 From: Cam Saul <cammsaul@gmail.com> Date: Mon, 8 Apr 2019 19:35:51 -0700 Subject: [PATCH] Don't set sessionVariables=sql_mode='ALLOW_INVALID_DATES' for MySQL/MariaDB [ci mysql] --- src/metabase/driver/mysql.clj | 5 +--- test/metabase/driver/mysql_test.clj | 46 +++++++++++++++++++---------- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/src/metabase/driver/mysql.clj b/src/metabase/driver/mysql.clj index 936b2d97dbc..2aa624dd6fd 100644 --- a/src/metabase/driver/mysql.clj +++ b/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 diff --git a/test/metabase/driver/mysql_test.clj b/test/metabase/driver/mysql_test.clj index 11733653c2b..4c51b3c6aba 100644 --- a/test/metabase/driver/mysql_test.clj +++ b/test/metabase/driver/mysql_test.clj @@ -1,5 +1,6 @@ (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 -- GitLab