Hotfix add multistatements param on mysql url (#17428)

Automatic Merge

store/sqlstore/store.go

@@ -8,7 +8,6 @@ import (
 	dbsql "database/sql"
 	"encoding/json"
 	"fmt"
-	"net/url"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -1364,7 +1363,10 @@ func (ss *SqlStore) migrate(direction migrationDirection) error {
 
 	// When WithInstance is used in golang-migrate, the underlying driver connections are not tracked.
 	// So we will have to open a fresh connection for migrations and explicitly close it when all is done.
-	dataSource := ss.appendMultipleStatementsFlag(*ss.settings.DataSource)
+	dataSource, err := ss.appendMultipleStatementsFlag(*ss.settings.DataSource)
+	if err != nil {
+		return err
+	}
 	conn := setupConnection("migrations", dataSource, ss.settings)
 	defer conn.Db.Close()
 
@@ -1422,22 +1424,20 @@ func (ss *SqlStore) migrate(direction migrationDirection) error {
 	return nil
 }
 
-func (ss *SqlStore) appendMultipleStatementsFlag(dataSource string) string {
+func (ss *SqlStore) appendMultipleStatementsFlag(dataSource string) (string, error) {
 	// We need to tell the MySQL driver that we want to use multiStatements
 	// in order to make migrations work.
 	if ss.DriverName() == model.DATABASE_DRIVER_MYSQL {
-		u, err := url.Parse(dataSource)
+		config, err := mysql.ParseDSN(dataSource)
 		if err != nil {
-			mlog.Critical("Invalid database url found", mlog.Err(err))
-			os.Exit(ExitGenericFailure)
+			return "", err
 		}
-		q := u.Query()
-		q.Set("multiStatements", "true")
-		u.RawQuery = q.Encode()
-		return u.String()
+
+		config.Params["multiStatements"] = "true"
+		return config.FormatDSN(), nil
 	}
 
-	return dataSource
+	return dataSource, nil
 }
 
 type mattermConverter struct{}

store/sqlstore/store_test.go

@@ -607,6 +607,44 @@ func TestReplicaLagQuery(t *testing.T) {
 	}
 }
 
+func TestAppendMultipleStatementsFlagMysql(t *testing.T) {
+	testCases := []struct {
+		Scenario    string
+		DSN         string
+		ExpectedDSN string
+		Driver      string
+	}{
+		{
+			"Should append multiStatements param to the DSN path with existing params",
+			"user:rand?&ompasswith@character@unix(/var/run/mysqld/mysqld.sock)/mattermost?writeTimeout=30s",
+			"user:rand?&ompasswith@character@unix(/var/run/mysqld/mysqld.sock)/mattermost?writeTimeout=30s&multiStatements=true",
+			model.DATABASE_DRIVER_MYSQL,
+		},
+		{
+			"Should append multiStatements param to the DSN path with no existing params",
+			"user:rand?&ompasswith@character@unix(/var/run/mysqld/mysqld.sock)/mattermost",
+			"user:rand?&ompasswith@character@unix(/var/run/mysqld/mysqld.sock)/mattermost?multiStatements=true",
+			model.DATABASE_DRIVER_MYSQL,
+		},
+		{
+			"Should not multiStatements param to the DSN when driver is not MySQL",
+			"user:rand?&ompasswith@character@unix(/var/run/mysqld/mysqld.sock)/mattermost",
+			"user:rand?&ompasswith@character@unix(/var/run/mysqld/mysqld.sock)/mattermost",
+			model.DATABASE_DRIVER_POSTGRES,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.Scenario, func(t *testing.T) {
+			t.Parallel()
+			store := &SqlStore{settings: &model.SqlSettings{DriverName: &tc.Driver, DataSource: &tc.DSN}}
+			res, err := store.appendMultipleStatementsFlag(*store.settings.DataSource)
+			require.NoError(t, err)
+			assert.Equal(t, tc.ExpectedDSN, res)
+		})
+	}
+}
+
 func makeSqlSettings(driver string) *model.SqlSettings {
 	switch driver {
 	case model.DATABASE_DRIVER_POSTGRES: