diff --git a/docker-compose.yml b/docker-compose.yml index b63bd55182a7d93f6d87576c49bbd939fd48366d..6ea3b8c6ce3f56cb8341cb3f00db27db35ba7e8d 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -85,7 +85,7 @@ services: volumes: - .:/pg_migrate/ - ./test/fixtures/mysql/:/docker-entrypoint-initdb.d/ - ports: ["3306:3306"] + ports: ["13306:3306"] environment: MYSQL_ROOT_PASSWORD: N0tSecret MYSQL_USER: sakila @@ -124,7 +124,7 @@ services: volumes: - .:/pg_migrate/ - ./test/fixtures/mariadb/:/docker-entrypoint-initdb.d/ - ports: ["3308:3306"] + ports: ["3306:3306"] environment: MARIADB_ROOT_PASSWORD: N0tSecret MARIADB_USER: sakila diff --git a/internal/mysql/audit.go b/internal/mysql/audit.go index a4d4dc0fbf9339676866bd5f31e2a6a42f1c9435..e6db3b54856fa4b86b259be782446bcb63d89604 100644 --- a/internal/mysql/audit.go +++ b/internal/mysql/audit.go @@ -10,6 +10,7 @@ import ( "gitlab.com/dalibo/pg_migrate/internal/catalog" "gitlab.com/dalibo/pg_migrate/internal/project" "gitlab.com/dalibo/transqlate/ast" + "gitlab.com/dalibo/transqlate/lexer" ) var scores project.Scores @@ -120,18 +121,30 @@ func (mdl *model) auditTable(table *catalog.Table) { c.Annotations = nil audit.Identifier(c, c.Name) audit.Expression(c, c.Default) - if c.Type.Name == "enum" { //nolint:staticcheck + + switch c.Type.Name { + case "enum": c.Annotate("type enum", scores.ColumnTypeEnum) - } else if c.Type.Name == "set" { + unquoted := lexer.UnquoteString(c.Default) + if unquoted != "" && !slices.Contains(c.Type.Values(), lexer.QuoteString(unquoted)) { + c.Annotate("non literral default value", scores.ColumnTypeEnumSetDefault) + } + case "set": c.Annotate("type set", scores.ColumnTypeSet) + if c.Default != "" && lexer.UnquoteString(c.Default) != "" { + unquoted := lexer.UnquoteString(c.Default) + if !IsValidDefaultSet(unquoted, c.Type.Values()) { + c.Annotate("non literral default value", scores.ColumnTypeEnumSetDefault) + } + } } - if c.Generated == "VIRTUAL" { + + switch c.Generated { + case "VIRTUAL": c.Annotate("virtual column", scores.VirtualColumn) - } - if c.Generated == "ALWAYS AS IDENTITY" { + case "ALWAYS AS IDENTITY": c.Score += scores.Sequence - } - if c.Generated == "ON UPDATE CURRENT_TIMESTAMP" { + case "ON UPDATE CURRENT_TIMESTAMP": c.Annotate("on update current timestamp", scores.ColumnOnUpdateCurrentTimestamp) } } diff --git a/internal/mysql/convert.go b/internal/mysql/convert.go index 4af0c513d67752d807e34a19e286c40c9e15c01c..31fdb821f5acd230391b93aeeb6dec9a90deab50 100644 --- a/internal/mysql/convert.go +++ b/internal/mysql/convert.go @@ -58,13 +58,27 @@ func convertEnumSet(t catalog.Table) catalog.Table { slog.Debug("Setting default enum value.", "value", values[0], "path", c.ObjectPath) break } - c.Default = lexer.QuoteString(c.Default) - if slices.Contains(values, c.Default) { - slog.Debug("Converting default enum value.", "value", c.Default, "path", c.ObjectPath) - t.Columns[i].Default = c.Default + quoted := lexer.QuoteString(c.Default) + if slices.Contains(values, quoted) { + t.Columns[i].Default = quoted + slog.Debug("Quoting default enum value.", "value", quoted, "path", c.ObjectPath) } case "set": check.Expression = fmt.Sprintf("%s <@ ARRAY[%s]", c.Name, items) + if (!c.Nullable && c.Default == "") || c.Default == "''" { + t.Columns[i].Default = "'{}'" + slog.Debug("Setting default set value.", "value", "'{}'", "path", c.ObjectPath) + break + } + if c.Default != "" { + // MariaDB returns the quoted default value, whereas MySQL does not. + unquoted := lexer.UnquoteString(c.Default) + if IsValidDefaultSet(unquoted, values) { + value := fmt.Sprintf("'{%s}'", unquoted) + t.Columns[i].Default = value + slog.Debug("Setting default set value.", "value", value, "path", c.ObjectPath) + } + } default: slog.Warn("Unexpected type with values.", "type", c.Type, "path", c.ObjectPath) continue diff --git a/internal/mysql/convert_test.go b/internal/mysql/convert_test.go new file mode 100644 index 0000000000000000000000000000000000000000..b796e5a8beb04d8660766085cbd7a76c00dacea0 --- /dev/null +++ b/internal/mysql/convert_test.go @@ -0,0 +1,218 @@ +package mysql + +import ( + "testing" + + "github.com/stretchr/testify/require" + "gitlab.com/dalibo/pg_migrate/internal/catalog" +) + +func TestConvertEnumNotNullable(t *testing.T) { + r := require.New(t) + table := catalog.Table{ + Name: "enum", + Columns: []catalog.Column{ + { + Name: "myEnumColumn", + Type: catalog.DataType{ + Name: "enum", + Params: []string{ + "'tuut'", + "'pouet'", + "'bip'", + }, + }, + Nullable: false, + }, + }, + } + table = convertEnumSet(table) + r.Equal("'tuut'", table.Columns[0].Default) + r.Equal("myEnumColumn in ('tuut', 'pouet', 'bip')", table.Checks[0].Expression) +} + +func TestConvertEnumNotNullableMySQL(t *testing.T) { + r := require.New(t) + table := catalog.Table{ + Name: "enum", + Columns: []catalog.Column{ + { + Name: "myEnumColumn", + Type: catalog.DataType{ + Name: "enum", + Params: []string{ + "'tuut'", + "'pouet'", + "'bip'", + }, + }, + Nullable: false, + Default: "bip", + }, + }, + } + table = convertEnumSet(table) + r.Equal("'bip'", table.Columns[0].Default) + r.Equal("myEnumColumn in ('tuut', 'pouet', 'bip')", table.Checks[0].Expression) +} + +func TestConvertEnumNotNullableMaria(t *testing.T) { + r := require.New(t) + table := catalog.Table{ + Name: "enum", + Columns: []catalog.Column{ + { + Name: "myEnumColumn", + Type: catalog.DataType{ + Name: "enum", + Params: []string{ + "'tuut'", + "'pouet'", + "'bip'", + }, + }, + Nullable: false, + Default: "'pouet'", + }, + }, + } + table = convertEnumSet(table) + r.Equal("'pouet'", table.Columns[0].Default) + r.Equal("myEnumColumn in ('tuut', 'pouet', 'bip')", table.Checks[0].Expression) +} + +func TestConvertEnumNonLitteral(t *testing.T) { + r := require.New(t) + table := catalog.Table{ + Name: "enum", + Columns: []catalog.Column{ + { + Name: "myEnumColumn", + Type: catalog.DataType{ + Name: "enum", + Params: []string{ + "'tuut'", + "'pouet'", + "'bip'", + }, + }, + Nullable: true, + Default: "maFonction(mondIdentifiant)", + }, + }, + } + table = convertEnumSet(table) + r.Equal("maFonction(mondIdentifiant)", table.Columns[0].Default) + r.Equal("myEnumColumn in ('tuut', 'pouet', 'bip')", table.Checks[0].Expression) +} + +func TestConvertSetNotNullable(t *testing.T) { + r := require.New(t) + table := catalog.Table{ + Name: "set", + Columns: []catalog.Column{ + { + Name: "mySetColumn", + Type: catalog.DataType{ + Name: "set", + Params: []string{ + "'tuut'", + "'pouet'", + "'bip'", + }, + }, + Nullable: false, + }, + }, + } + table = convertEnumSet(table) + r.Equal("'{}'", table.Columns[0].Default) + r.Equal("mySetColumn <@ ARRAY['tuut', 'pouet', 'bip']", table.Checks[0].Expression) +} + +func TestConvertSetNotNullableMaria(t *testing.T) { + r := require.New(t) + table := catalog.Table{ + Name: "set", + Columns: []catalog.Column{ + { + Name: "mySetColumn", + Type: catalog.DataType{ + Name: "set", + Params: []string{ + "'tuut'", + "'pouet'", + "'bip'", + }, + }, + Nullable: false, + Default: "pouet, bip", + }, + }, + } + table = convertEnumSet(table) + r.Equal("'{pouet, bip}'", table.Columns[0].Default) + r.Equal("mySetColumn <@ ARRAY['tuut', 'pouet', 'bip']", table.Checks[0].Expression) +} + +func TestConvertSetemptyString(t *testing.T) { + r := require.New(t) + table := catalog.Table{ + Name: "set", + Columns: []catalog.Column{ + { + Name: "mySetColumn", + Type: catalog.DataType{ + Name: "set", + Params: []string{ + "'tuut'", + "'pouet'", + "'bip'", + }, + }, + Nullable: false, + Default: "''", + }, + }, + } + table = convertEnumSet(table) + r.Equal("'{}'", table.Columns[0].Default) + r.Equal("mySetColumn <@ ARRAY['tuut', 'pouet', 'bip']", table.Checks[0].Expression) +} + +func TestConvertEnumSetNullable(t *testing.T) { + r := require.New(t) + table := catalog.Table{ + Name: "set", + Columns: []catalog.Column{ + { + Name: "myEnumColumn", + Type: catalog.DataType{ + Name: "enum", + Params: []string{ + "'tuut'", + "'pouet'", + "'bip'", + }, + }, + Nullable: true, + }, + { + Name: "mySetColumn", + Type: catalog.DataType{ + Name: "set", + Params: []string{ + "'tuut'", + "'pouet'", + "'bip'", + }, + }, + Nullable: true, + }, + }, + } + + table = convertEnumSet(table) + r.Equal("", table.Columns[0].Default) + r.Equal("", table.Columns[1].Default) +} diff --git a/internal/mysql/sql/columns.sql b/internal/mysql/sql/columns.sql index 1496b1f298fabbb1902b7cbe2e60084d5201f73b..a79b10769cfb72a9e125ec51ea73396894410193 100644 --- a/internal/mysql/sql/columns.sql +++ b/internal/mysql/sql/columns.sql @@ -4,7 +4,13 @@ SELECT c.table_schema, c.table_name, column_name, column_comment, NULL AS "precision", NULL AS "scale", is_nullable = 'YES' AS nullable, - COALESCE(NULLIF(generation_expression,''), column_default), + COALESCE( + NULLIF(generation_expression,''), + CASE + WHEN column_default = '' THEN '''''' + ELSE column_default + END + ), CASE extra WHEN 'STORED GENERATED' THEN 'STORED' WHEN 'VIRTUAL GENERATED' THEN 'VIRTUAL' diff --git a/internal/mysql/tables.go b/internal/mysql/tables.go index b44d54369dc84a32f2f5a8d800aa91d45796a87d..d4eef0cb4856cda7185b06452b97f6e3ad5f368a 100644 --- a/internal/mysql/tables.go +++ b/internal/mysql/tables.go @@ -4,12 +4,14 @@ import ( "context" "database/sql" "log/slog" + "slices" "strings" "gitlab.com/dalibo/pg_migrate/internal/catalog" "gitlab.com/dalibo/pg_migrate/internal/database" "gitlab.com/dalibo/pg_migrate/internal/errorlist" "gitlab.com/dalibo/pg_migrate/internal/fetch" + "gitlab.com/dalibo/transqlate/lexer" ) func (mdl *model) inspectTables(ctx context.Context) error { @@ -212,3 +214,18 @@ func rowToSubPartition(rows *sql.Rows) (a catalog.Association, err error) { a.Component = p return a, err } + +// IsValidDefaultSet returns true if defaultValue is a valid default set value +// +// defaultValue must be unquoted, values contains possible set values +func IsValidDefaultSet(defaultValue string, values []string) bool { + defaultValues := strings.Split(defaultValue, ",") + contains := true + for _, d := range defaultValues { + if !slices.Contains(values, lexer.QuoteString(strings.TrimSpace(d))) { + contains = false + break + } + } + return contains +} diff --git a/internal/project/scores.go b/internal/project/scores.go index 6b1cf5dd2251c07c04e896d847f4c98d98c8313d..5baccca4cebbbe43d9621319a2a3b7ba9cce82b7 100644 --- a/internal/project/scores.go +++ b/internal/project/scores.go @@ -6,6 +6,7 @@ type Scores struct { Column float32 ColumnTypeEnum float32 ColumnTypeSet float32 + ColumnTypeEnumSetDefault float32 ColumnTypeLarge float32 ColumnMissingPrecision float32 ColumnNegativeScale float32 @@ -58,6 +59,7 @@ func newScores() Scores { Column: 0.1, ColumnTypeEnum: 0.5, ColumnTypeSet: 2, + ColumnTypeEnumSetDefault: 0.1, ColumnTypeLarge: 1, ColumnMissingPrecision: 1, ColumnNegativeScale: 1, diff --git a/test/fixtures/mariadb/00-extra.sql b/test/fixtures/mariadb/00-extra.sql index c77f480383e0539d77034659bb70db822dcfe377..2d9fd8ec5c29304bc8798abf3c4b34bf65d5f33e 100644 --- a/test/fixtures/mariadb/00-extra.sql +++ b/test/fixtures/mariadb/00-extra.sql @@ -3,6 +3,7 @@ CREATE TABLE x_table ( name VARCHAR(50) NOT NULL, upper_name VARCHAR(50) GENERATED ALWAYS AS (UPPER(name)) VIRTUAL, lower_name VARCHAR(50) GENERATED ALWAYS AS (LOWER(name)) STORED, + category SET('cat0','cat1','cat2') NOT NULL DEFAULT '', CONSTRAINT CHK_MixedCase_id CHECK (id BETWEEN 1 AND 10), CONSTRAINT x_table_uk UNIQUE (id, name) ); diff --git a/test/fixtures/mysql/00-extra.sql b/test/fixtures/mysql/00-extra.sql index 8e4c9ef9229c177d321970d4a850cf2154986067..9b66d8c147738d9ff905eac67f4ce6f1b2cc3de9 100644 --- a/test/fixtures/mysql/00-extra.sql +++ b/test/fixtures/mysql/00-extra.sql @@ -3,6 +3,7 @@ CREATE TABLE x_table ( name VARCHAR(50) NOT NULL, upper_name VARCHAR(50) GENERATED ALWAYS AS (UPPER(name)) VIRTUAL, lower_name VARCHAR(50) GENERATED ALWAYS AS (LOWER(name)) STORED, + category SET('cat0','cat1','cat2') NOT NULL DEFAULT '', CONSTRAINT CHK_MixedCase_id CHECK (id BETWEEN 1 AND 10), CONSTRAINT x_table_uk UNIQUE (id, name) );