From fb452d85659dd61bd7a91de2202f44e174d0613e Mon Sep 17 00:00:00 2001 From: Claudio Costa Date: Tue, 8 Jun 2021 08:37:43 +0200 Subject: [PATCH] Improve column default utilities (#17722) --- store/sqlstore/store.go | 27 +++++++-- store/sqlstore/store_test.go | 111 +++++++++++++++++++++++++++++++++++ store/sqlstore/upgrade.go | 17 +++--- 3 files changed, 142 insertions(+), 13 deletions(-) diff --git a/store/sqlstore/store.go b/store/sqlstore/store.go index e8a6930f4c..5da80a96f0 100644 --- a/store/sqlstore/store.go +++ b/store/sqlstore/store.go @@ -909,7 +909,23 @@ func (ss *SqlStore) AlterColumnTypeIfExists(tableName string, columnName string, return true } -func (ss *SqlStore) AlterColumnDefaultIfExists(tableName string, columnName string, mySqlColDefault *string, postgresColDefault *string) bool { +func (ss *SqlStore) RemoveDefaultIfColumnExists(tableName, columnName string) bool { + if !ss.DoesColumnExist(tableName, columnName) { + return false + } + + _, err := ss.GetMaster().ExecNoTimeout("ALTER TABLE " + tableName + " ALTER COLUMN " + columnName + " DROP DEFAULT") + if err != nil { + mlog.Critical("Failed to drop column default", mlog.String("table", tableName), mlog.String("column", columnName), mlog.Err(err)) + time.Sleep(time.Second) + os.Exit(ExitGenericFailure) + return false + } + + return true +} + +func (ss *SqlStore) AlterDefaultIfColumnExists(tableName string, columnName string, mySqlColDefault *string, postgresColDefault *string) bool { if !ss.DoesColumnExist(tableName, columnName) { return false } @@ -938,15 +954,14 @@ func (ss *SqlStore) AlterColumnDefaultIfExists(tableName string, columnName stri return false } - var err error if defaultValue == "" { - _, err = ss.GetMaster().ExecNoTimeout("ALTER TABLE " + tableName + " ALTER COLUMN " + columnName + " DROP DEFAULT") - } else { - _, err = ss.GetMaster().ExecNoTimeout("ALTER TABLE " + tableName + " ALTER COLUMN " + columnName + " SET DEFAULT " + defaultValue) + defaultValue = "''" } + query := "ALTER TABLE " + tableName + " ALTER COLUMN " + columnName + " SET DEFAULT " + defaultValue + _, err := ss.GetMaster().ExecNoTimeout(query) if err != nil { - mlog.Critical("Failed to alter column", mlog.String("table", tableName), mlog.String("column", columnName), mlog.String("default value", defaultValue), mlog.Err(err)) + mlog.Critical("Failed to alter column default", mlog.String("table", tableName), mlog.String("column", columnName), mlog.String("default value", defaultValue), mlog.Err(err)) time.Sleep(time.Second) os.Exit(ExitGenericFailure) return false diff --git a/store/sqlstore/store_test.go b/store/sqlstore/store_test.go index 309a7a1d48..a3e51aa7c3 100644 --- a/store/sqlstore/store_test.go +++ b/store/sqlstore/store_test.go @@ -739,3 +739,114 @@ func TestMySQLReadTimeout(t *testing.T) { _, err = store.GetMaster().ExecNoTimeout(`SELECT SLEEP(3)`) require.NoError(t, err) } + +func TestAlterDefaultIfColumnExists(t *testing.T) { + StoreTest(t, func(t *testing.T, ss store.Store) { + var query string + def := new(string) + sqlStore := ss.(*SqlStore) + + t.Run("non existent table", func(t *testing.T) { + ok := sqlStore.AlterDefaultIfColumnExists("NotExistent", "NotExistent", nil, nil) + require.False(t, ok) + }) + + t.Run("non existent column", func(t *testing.T) { + ok := sqlStore.AlterDefaultIfColumnExists("Posts", "NotExistent", nil, nil) + require.False(t, ok) + }) + + t.Run("empty string", func(t *testing.T) { + ok := sqlStore.AlterDefaultIfColumnExists("Posts", "Id", model.NewString(""), model.NewString("")) + require.True(t, ok) + + if sqlStore.DriverName() == model.DATABASE_DRIVER_MYSQL { + query = `SELECT column_default + FROM information_schema.columns + WHERE table_schema = DATABASE() + AND table_name = 'Posts' + AND column_name = 'Id'` + } else if sqlStore.DriverName() == model.DATABASE_DRIVER_POSTGRES { + query = `SELECT column_default + FROM information_schema.columns + WHERE table_name = 'posts' + AND column_name = 'id'` + } + + err := sqlStore.GetMaster().SelectOne(&def, query) + require.NoError(t, err) + require.NotNil(t, def) + if sqlStore.DriverName() == model.DATABASE_DRIVER_MYSQL { + require.Equal(t, "", *def) + } else if sqlStore.DriverName() == model.DATABASE_DRIVER_POSTGRES { + require.Equal(t, "''::character varying", *def) + } + }) + + t.Run("nil input", func(t *testing.T) { + ok := sqlStore.AlterDefaultIfColumnExists("Posts", "Id", nil, nil) + require.True(t, ok) + + err := sqlStore.GetMaster().SelectOne(&def, query) + require.NoError(t, err) + require.NotNil(t, def) + if sqlStore.DriverName() == model.DATABASE_DRIVER_MYSQL { + require.Equal(t, "", *def) + } else if sqlStore.DriverName() == model.DATABASE_DRIVER_POSTGRES { + require.Equal(t, "''::character varying", *def) + } + }) + + t.Run("remove", func(t *testing.T) { + ok := sqlStore.RemoveDefaultIfColumnExists("Posts", "Id") + require.True(t, ok) + + err := sqlStore.GetMaster().SelectOne(&def, query) + require.NoError(t, err) + require.Nil(t, def) + }) + + t.Run("string default", func(t *testing.T) { + ok := sqlStore.AlterDefaultIfColumnExists("Posts", "Id", model.NewString("'test'"), model.NewString("'test'")) + require.True(t, ok) + + err := sqlStore.GetMaster().SelectOne(&def, query) + require.NoError(t, err) + require.NotNil(t, def) + if sqlStore.DriverName() == model.DATABASE_DRIVER_MYSQL { + require.Equal(t, "test", *def) + } else if sqlStore.DriverName() == model.DATABASE_DRIVER_POSTGRES { + require.Equal(t, "'test'::character varying", *def) + } + + ok = sqlStore.RemoveDefaultIfColumnExists("Posts", "Id") + require.True(t, ok) + }) + + t.Run("int default", func(t *testing.T) { + ok := sqlStore.AlterDefaultIfColumnExists("Posts", "UpdateAt", model.NewString("0"), model.NewString("0")) + require.True(t, ok) + + if sqlStore.DriverName() == model.DATABASE_DRIVER_MYSQL { + query = `SELECT column_default + FROM information_schema.columns + WHERE table_schema = DATABASE() + AND table_name = 'Posts' + AND column_name = 'UpdateAt'` + } else if sqlStore.DriverName() == model.DATABASE_DRIVER_POSTGRES { + query = `SELECT column_default + FROM information_schema.columns + WHERE table_name = 'posts' + AND column_name = 'updateat'` + } + + err := sqlStore.GetMaster().SelectOne(&def, query) + require.NoError(t, err) + require.NotNil(t, def) + require.Equal(t, "0", *def) + + ok = sqlStore.RemoveDefaultIfColumnExists("Posts", "UpdateAt") + require.True(t, ok) + }) + }) +} diff --git a/store/sqlstore/upgrade.go b/store/sqlstore/upgrade.go index 17c12ed918..c490a19bb9 100644 --- a/store/sqlstore/upgrade.go +++ b/store/sqlstore/upgrade.go @@ -662,9 +662,12 @@ func upgradeDatabaseToVersion58(sqlStore *SqlStore) { sqlStore.AlterColumnTypeIfExists("OutgoingWebhooks", "Description", "text", "VARCHAR(500)") sqlStore.AlterColumnTypeIfExists("IncomingWebhooks", "Description", "text", "VARCHAR(500)") sqlStore.AlterColumnTypeIfExists("OutgoingWebhooks", "IconURL", "text", "VARCHAR(1024)") - sqlStore.AlterColumnDefaultIfExists("OutgoingWebhooks", "Username", model.NewString("NULL"), model.NewString("")) - sqlStore.AlterColumnDefaultIfExists("OutgoingWebhooks", "IconURL", nil, model.NewString("")) - sqlStore.AlterColumnDefaultIfExists("PluginKeyValueStore", "ExpireAt", model.NewString("NULL"), model.NewString("NULL")) + sqlStore.RemoveDefaultIfColumnExists("OutgoingWebhooks", "Username") + if sqlStore.DriverName() == model.DATABASE_DRIVER_POSTGRES { + sqlStore.RemoveDefaultIfColumnExists("OutgoingWebhooks", "IconURL") + } + sqlStore.AlterDefaultIfColumnExists("OutgoingWebhooks", "Username", model.NewString("NULL"), nil) + sqlStore.AlterDefaultIfColumnExists("PluginKeyValueStore", "ExpireAt", model.NewString("NULL"), model.NewString("NULL")) saveSchemaVersion(sqlStore, Version580) } @@ -938,9 +941,9 @@ func precheckMigrationToVersion528(sqlStore *SqlStore) error { func upgradeDatabaseToVersion529(sqlStore *SqlStore) { if shouldPerformUpgrade(sqlStore, Version5281, Version5290) { sqlStore.AlterColumnTypeIfExists("SidebarCategories", "Id", "VARCHAR(128)", "VARCHAR(128)") - sqlStore.AlterColumnDefaultIfExists("SidebarCategories", "Id", model.NewString(""), nil) + sqlStore.RemoveDefaultIfColumnExists("SidebarCategories", "Id") sqlStore.AlterColumnTypeIfExists("SidebarChannels", "CategoryId", "VARCHAR(128)", "VARCHAR(128)") - sqlStore.AlterColumnDefaultIfExists("SidebarChannels", "CategoryId", model.NewString(""), nil) + sqlStore.RemoveDefaultIfColumnExists("SidebarChannels", "CategoryId") sqlStore.CreateColumnIfNotExistsNoDefault("Threads", "ChannelId", "VARCHAR(26)", "VARCHAR(26)") @@ -1091,7 +1094,7 @@ func rootCountMigration(sqlStore *SqlStore) { msgCountRootExists := sqlStore.DoesColumnExist("ChannelMembers", "MsgCountRoot") sqlStore.CreateColumnIfNotExists("ChannelMembers", "MentionCountRoot", "bigint", "bigint", "0") - sqlStore.AlterColumnDefaultIfExists("ChannelMembers", "MentionCountRoot", model.NewString("0"), model.NewString("0")) + sqlStore.AlterDefaultIfColumnExists("ChannelMembers", "MentionCountRoot", model.NewString("0"), model.NewString("0")) mentionCountRootCTE := ` SELECT ChannelId, COALESCE(SUM(UnreadMentions), 0) AS UnreadMentions, UserId @@ -1126,7 +1129,7 @@ func rootCountMigration(sqlStore *SqlStore) { defer sqlStore.RemoveColumnIfExists("Channels", "LastRootPostAt") sqlStore.CreateColumnIfNotExists("ChannelMembers", "MsgCountRoot", "bigint", "bigint", "0") - sqlStore.AlterColumnDefaultIfExists("ChannelMembers", "MsgCountRoot", model.NewString("0"), model.NewString("0")) + sqlStore.AlterDefaultIfColumnExists("ChannelMembers", "MsgCountRoot", model.NewString("0"), model.NewString("0")) forceIndex := "" if sqlStore.DriverName() == model.DATABASE_DRIVER_MYSQL {