Repository: schemahero/schemahero
Pull request #626 (2021-12-08 23:27:24)

Respect default charset and collation in mysql
Signed-off-by: Marc Campbell <marc.e.campbell@gmail.com>


Patch:
diff --git a/config/crds/v1/schemas.schemahero.io_migrations.yaml b/config/crds/v1/schemas.schemahero.io_migrations.yaml
index cda6915c..26635be8 100644
--- a/config/crds/v1/schemas.schemahero.io_migrations.yaml
+++ b/config/crds/v1/schemas.schemahero.io_migrations.yaml
@@ -64,7 +64,6 @@ spec:
               tableNamespace:
                 type: string
             required:
-            - databaseName
             - tableName
             - tableNamespace
             type: object
@@ -97,8 +96,6 @@ spec:
               rejectedAt:
                 format: int64
                 type: integer
-            required:
-            - phase
             type: object
         type: object
     served: true
diff --git a/pkg/apis/databases/v1alpha4/zz_generated.deepcopy.go b/pkg/apis/databases/v1alpha4/zz_generated.deepcopy.go
index ee2d247c..6347340d 100644
--- a/pkg/apis/databases/v1alpha4/zz_generated.deepcopy.go
+++ b/pkg/apis/databases/v1alpha4/zz_generated.deepcopy.go
@@ -1,3 +1,4 @@
+//go:build !ignore_autogenerated
 // +build !ignore_autogenerated
 
 /*
diff --git a/pkg/apis/schemas/v1alpha4/zz_generated.deepcopy.go b/pkg/apis/schemas/v1alpha4/zz_generated.deepcopy.go
index 73486bd3..98ead54f 100644
--- a/pkg/apis/schemas/v1alpha4/zz_generated.deepcopy.go
+++ b/pkg/apis/schemas/v1alpha4/zz_generated.deepcopy.go
@@ -1,3 +1,4 @@
+//go:build !ignore_autogenerated
 // +build !ignore_autogenerated
 
 /*
diff --git a/pkg/database/mysql/alter.go b/pkg/database/mysql/alter.go
index 96c32c5a..f3af1690 100644
--- a/pkg/database/mysql/alter.go
+++ b/pkg/database/mysql/alter.go
@@ -5,7 +5,7 @@ import (
 	"github.com/schemahero/schemahero/pkg/database/types"
 )
 
-func AlterColumnStatements(tableName string, primaryKeys []string, desiredColumns []*schemasv1alpha4.MysqlTableColumn, existingColumn *types.Column) ([]string, error) {
+func AlterColumnStatements(tableName string, primaryKeys []string, desiredColumns []*schemasv1alpha4.MysqlTableColumn, existingColumn *types.Column, defaultCharset string, defaultCollation string) ([]string, error) {
 	// this could be an alter or a drop column command
 	for _, desiredColumn := range desiredColumns {
 		if desiredColumn.Name == existingColumn.Name {
@@ -26,7 +26,7 @@ func AlterColumnStatements(tableName string, primaryKeys []string, desiredColumn
 				ensureColumnConstraintsNotNullTrue(column)
 			}
 
-			if columnsMatch(*existingColumn, *column) {
+			if columnsMatch(*existingColumn, *column, defaultCharset, defaultCollation) {
 				return []string{}, nil
 			}
 
@@ -45,11 +45,25 @@ func AlterColumnStatements(tableName string, primaryKeys []string, desiredColumn
 	}.DDL(), nil
 }
 
-func columnsMatch(col1 types.Column, col2 types.Column) bool {
+func columnsMatch(col1 types.Column, col2 types.Column, defaultCharset string, defaultCollation string) bool {
 	if col1.DataType != col2.DataType {
 		return false
 	}
 
+	if col1.Charset == "" {
+		col1.Charset = defaultCharset
+	}
+	if col2.Charset == "" {
+		col2.Charset = defaultCharset
+	}
+	if col1.Collation == "" {
+		col1.Collation = defaultCollation
+	}
+	if col2.Collation == "" {
+		col2.Collation = defaultCollation
+	}
+
+	// now that we've applied defaults, let's see if they actually are different
 	if col1.Charset != col2.Charset {
 		return false
 	}
diff --git a/pkg/database/mysql/alter_statements.go b/pkg/database/mysql/alter_statements.go
index 92d318ca..1f511be2 100644
--- a/pkg/database/mysql/alter_statements.go
+++ b/pkg/database/mysql/alter_statements.go
@@ -42,12 +42,20 @@ func (s AlterModifyColumnStatement) ddl(useConstraintsFromExistingColumn bool) [
 	}
 
 	if s.Column.Charset != s.ExistingColumn.Charset || s.Column.Collation != s.ExistingColumn.Collation {
-		stmt := fmt.Sprintf("character set %s", s.Column.Charset)
+		stmt := ""
+		if s.Column.Charset != "" {
+			stmt = fmt.Sprintf("character set %s", s.Column.Charset)
+		}
 		if s.Column.Collation != "" {
-			stmt = fmt.Sprintf("%s collate %s", stmt, s.Column.Collation)
+			if len(stmt) > 0 {
+				stmt = stmt + " "
+			}
+			stmt = fmt.Sprintf("%scollate %s", stmt, s.Column.Collation)
 		}
 
-		stmts = append(stmts, stmt)
+		if len(stmt) > 0 {
+			stmts = append(stmts, stmt)
+		}
 	}
 
 	if useConstraintsFromExistingColumn {
diff --git a/pkg/database/mysql/deploy.go b/pkg/database/mysql/deploy.go
index 462498f6..c78a317a 100644
--- a/pkg/database/mysql/deploy.go
+++ b/pkg/database/mysql/deploy.go
@@ -213,7 +213,51 @@ WHERE schema_name = ?`
 	}, nil
 }
 
+// getDefaultCharsetAndCollationForTable will return the applied charset, collation for the specifed table
+func getDefaultCharsetAndCollationForTable(m *MysqlConnection, tableName string) (string, string, error) {
+	query := `SELECT default_character_set_name, default_collation_name FROM information_schema.SCHEMATA WHERE schema_name = ?`
+	row := m.db.QueryRow(query, m.databaseName)
+
+	defaultCharset := ""
+	defaultCollation := ""
+	if err := row.Scan(&defaultCharset, &defaultCollation); err != nil {
+		return "", "", errors.Wrap(err, "scan default charset and collation")
+	}
+
+	query = `select
+t.TABLE_COLLATION,
+c.character_set_name FROM information_schema.TABLES t,
+information_schema.COLLATION_CHARACTER_SET_APPLICABILITY c
+WHERE c.collation_name = t.table_collation
+AND t.table_schema = ?
+AND t.table_name = ?`
+	row = m.db.QueryRow(query, m.databaseName, tableName)
+
+	tableCharset := sql.NullString{}
+	tableCollation := sql.NullString{}
+	if err := row.Scan(&tableCollation, &tableCharset); err != nil {
+		return "", "", errors.Wrap(err, "scan table charset and collation")
+	}
+
+	mostSpecificCharset := defaultCharset
+	if tableCharset.Valid {
+		mostSpecificCharset = tableCharset.String
+	}
+
+	mostSpecificCollation := defaultCollation
+	if tableCollation.Valid {
+		mostSpecificCollation = tableCollation.String
+	}
+
+	return mostSpecificCharset, mostSpecificCollation, nil
+}
+
 func buildColumnStatements(m *MysqlConnection, tableName string, mysqlTableSchema *schemasv1alpha4.MysqlTableSchema) ([]string, error) {
+	defaultCharset, defaultCollation, err := getDefaultCharsetAndCollationForTable(m, tableName)
+	if err != nil {
+		return nil, errors.Wrap(err, "get default charset and collation")
+	}
+
 	query := `select
 COLUMN_NAME, COLUMN_DEFAULT, IS_NULLABLE, EXTRA, COLUMN_TYPE, CHARACTER_MAXIMUM_LENGTH, CHARACTER_SET_NAME, COLLATION_NAME
 from information_schema.COLUMNS
@@ -289,7 +333,7 @@ where TABLE_NAME = ?`
 			existingColumn.ColumnDefault = &columnDefault.String
 		}
 
-		columnStatement, err := AlterColumnStatements(tableName, mysqlTableSchema.PrimaryKey, mysqlTableSchema.Columns, &existingColumn)
+		columnStatement, err := AlterColumnStatements(tableName, mysqlTableSchema.PrimaryKey, mysqlTableSchema.Columns, &existingColumn, defaultCharset, defaultCollation)
 		if err != nil {
 			return nil, errors.Wrap(err, "failed to create alter column statement")
 		}
diff --git a/pkg/installer/assets/schemas.schemahero.io_migrations.yaml b/pkg/installer/assets/schemas.schemahero.io_migrations.yaml
index cda6915c..26635be8 100644
--- a/pkg/installer/assets/schemas.schemahero.io_migrations.yaml
+++ b/pkg/installer/assets/schemas.schemahero.io_migrations.yaml
@@ -64,7 +64,6 @@ spec:
               tableNamespace:
                 type: string
             required:
-            - databaseName
             - tableName
             - tableNamespace
             type: object
@@ -97,8 +96,6 @@ spec:
               rejectedAt:
                 format: int64
                 type: integer
-            required:
-            - phase
             type: object
         type: object
     served: true


Test patch:
diff --git a/integration/tests/mysql/no-modify-charset/specs/projects.yaml b/integration/tests/mysql/no-modify-charset/specs/projects.yaml
index c30fa493..8a126e1c 100644
--- a/integration/tests/mysql/no-modify-charset/specs/projects.yaml
+++ b/integration/tests/mysql/no-modify-charset/specs/projects.yaml
@@ -14,5 +14,4 @@ schema:
         type: text (65535)
         constraints:
           notNull: false
-      - name: newcol
-        type: int
+
diff --git a/pkg/database/mysql/alter_test.go b/pkg/database/mysql/alter_test.go
index c355af5b..4a4052fd 100644
--- a/pkg/database/mysql/alter_test.go
+++ b/pkg/database/mysql/alter_test.go
@@ -20,6 +20,8 @@ func Test_AlterColumnStatment(t *testing.T) {
 		desiredColumns     []*schemasv1alpha4.MysqlTableColumn
 		existingColumn     *types.Column
 		expectedStatements []string
+		defaultCharset     string
+		defaultCollation   string
 	}{
 		{
 			name:      "no change",
@@ -336,7 +338,7 @@ func Test_AlterColumnStatment(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			req := require.New(t)
 
-			generatedStatements, err := AlterColumnStatements(test.tableName, []string{}, test.desiredColumns, test.existingColumn)
+			generatedStatements, err := AlterColumnStatements(test.tableName, []string{}, test.desiredColumns, test.existingColumn, test.defaultCharset, test.defaultCollation)
 			req.NoError(err)
 			assert.Equal(t, test.expectedStatements, generatedStatements)
 		})