From da2efe437c968c528618d020a77327970c7ab1d9 Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Wed, 11 Dec 2024 12:09:50 +0100 Subject: [PATCH 1/6] sql2pgroll: Support set and drop column defaults --- pkg/sql2pgroll/alter_table.go | 23 +++++++++++++++++++++-- pkg/sql2pgroll/alter_table_test.go | 12 ++++++++++++ pkg/sql2pgroll/expect/alter_column.go | 18 ++++++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/pkg/sql2pgroll/alter_table.go b/pkg/sql2pgroll/alter_table.go index 915429ea..38f6a063 100644 --- a/pkg/sql2pgroll/alter_table.go +++ b/pkg/sql2pgroll/alter_table.go @@ -5,6 +5,7 @@ package sql2pgroll import ( "fmt" + "github.com/oapi-codegen/nullable" pgq "github.com/pganalyze/pg_query_go/v6" "github.com/xataio/pgroll/pkg/migrations" @@ -38,6 +39,8 @@ func convertAlterTableStmt(stmt *pgq.AlterTableStmt) (migrations.Operations, err op, err = convertAlterTableAddConstraint(stmt, alterTableCmd) case pgq.AlterTableType_AT_DropColumn: op, err = convertAlterTableDropColumn(stmt, alterTableCmd) + case pgq.AlterTableType_AT_ColumnDefault: + op, err = convertAlterTableSetColumnDefault(stmt, alterTableCmd) } if err != nil { @@ -158,11 +161,27 @@ func convertAlterTableAddUniqueConstraint(stmt *pgq.AlterTableStmt, constraint * }, nil } -// convertAlterTableDropColumn converts SQL statements like: +// convertAlterTableSetColumnDefault converts SQL statements like: // -// `ALTER TABLE foo DROP COLUMN bar +// `ALTER TABLE foo COLUMN bar SET DEFAULT 'foo' +// `ALTER TABLE foo COLUMN bar SET DEFAULT null +// `ALTER TABLE foo COLUMN bar DROP DEFAULT // // to an OpDropColumn operation. +func convertAlterTableSetColumnDefault(stmt *pgq.AlterTableStmt, cmd *pgq.AlterTableCmd) (migrations.Operation, error) { + def := nullable.NewNullNullable[string]() + if val := cmd.GetDef().GetAConst().GetSval(); val != nil { + def.Set(val.Sval) + } + return &migrations.OpAlterColumn{ + Table: stmt.GetRelation().GetRelname(), + Column: cmd.GetName(), + Default: def, + Down: PlaceHolderSQL, + Up: PlaceHolderSQL, + }, nil +} + func convertAlterTableDropColumn(stmt *pgq.AlterTableStmt, cmd *pgq.AlterTableCmd) (migrations.Operation, error) { if !canConvertDropColumn(cmd) { return nil, nil diff --git a/pkg/sql2pgroll/alter_table_test.go b/pkg/sql2pgroll/alter_table_test.go index 144b220b..68a4b52f 100644 --- a/pkg/sql2pgroll/alter_table_test.go +++ b/pkg/sql2pgroll/alter_table_test.go @@ -36,6 +36,18 @@ func TestConvertAlterTableStatements(t *testing.T) { sql: "ALTER TABLE foo ALTER COLUMN a TYPE text", expectedOp: expect.AlterColumnOp3, }, + { + sql: "ALTER TABLE foo ALTER COLUMN bar SET DEFAULT 'baz'", + expectedOp: expect.AlterColumnOp5, + }, + { + sql: "ALTER TABLE foo ALTER COLUMN bar DROP DEFAULT", + expectedOp: expect.AlterColumnOp6, + }, + { + sql: "ALTER TABLE foo ALTER COLUMN bar SET DEFAULT null", + expectedOp: expect.AlterColumnOp6, + }, { sql: "ALTER TABLE foo ADD CONSTRAINT bar UNIQUE (a)", expectedOp: expect.CreateConstraintOp1, diff --git a/pkg/sql2pgroll/expect/alter_column.go b/pkg/sql2pgroll/expect/alter_column.go index ec9b0acc..7b2af1eb 100644 --- a/pkg/sql2pgroll/expect/alter_column.go +++ b/pkg/sql2pgroll/expect/alter_column.go @@ -3,6 +3,8 @@ package expect import ( + "github.com/oapi-codegen/nullable" + "github.com/xataio/pgroll/pkg/migrations" "github.com/xataio/pgroll/pkg/sql2pgroll" ) @@ -37,6 +39,22 @@ var AlterColumnOp4 = &migrations.OpAlterColumn{ Name: ptr("b"), } +var AlterColumnOp5 = &migrations.OpAlterColumn{ + Table: "foo", + Column: "bar", + Default: nullable.NewNullableWithValue("baz"), + Up: sql2pgroll.PlaceHolderSQL, + Down: sql2pgroll.PlaceHolderSQL, +} + +var AlterColumnOp6 = &migrations.OpAlterColumn{ + Table: "foo", + Column: "bar", + Default: nullable.NewNullNullable[string](), + Up: sql2pgroll.PlaceHolderSQL, + Down: sql2pgroll.PlaceHolderSQL, +} + func ptr[T any](v T) *T { return &v } From 1834fc44330af6ea65b03a9098de55aacf1ccfd5 Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Wed, 11 Dec 2024 12:12:29 +0100 Subject: [PATCH 2/6] Use backticks --- pkg/sql2pgroll/alter_table.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/sql2pgroll/alter_table.go b/pkg/sql2pgroll/alter_table.go index 38f6a063..14818d3e 100644 --- a/pkg/sql2pgroll/alter_table.go +++ b/pkg/sql2pgroll/alter_table.go @@ -163,9 +163,9 @@ func convertAlterTableAddUniqueConstraint(stmt *pgq.AlterTableStmt, constraint * // convertAlterTableSetColumnDefault converts SQL statements like: // -// `ALTER TABLE foo COLUMN bar SET DEFAULT 'foo' -// `ALTER TABLE foo COLUMN bar SET DEFAULT null -// `ALTER TABLE foo COLUMN bar DROP DEFAULT +// `ALTER TABLE foo COLUMN bar SET DEFAULT 'foo'` +// `ALTER TABLE foo COLUMN bar SET DEFAULT null` +// `ALTER TABLE foo COLUMN bar DROP DEFAULT` // // to an OpDropColumn operation. func convertAlterTableSetColumnDefault(stmt *pgq.AlterTableStmt, cmd *pgq.AlterTableCmd) (migrations.Operation, error) { From 89ce4f7d542733fdc8ab282d6808992b636505f1 Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Wed, 11 Dec 2024 12:13:39 +0100 Subject: [PATCH 3/6] Fix comment --- pkg/sql2pgroll/alter_table.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sql2pgroll/alter_table.go b/pkg/sql2pgroll/alter_table.go index 14818d3e..1f6f559e 100644 --- a/pkg/sql2pgroll/alter_table.go +++ b/pkg/sql2pgroll/alter_table.go @@ -167,7 +167,7 @@ func convertAlterTableAddUniqueConstraint(stmt *pgq.AlterTableStmt, constraint * // `ALTER TABLE foo COLUMN bar SET DEFAULT null` // `ALTER TABLE foo COLUMN bar DROP DEFAULT` // -// to an OpDropColumn operation. +// to an OpAlterColumn operation. func convertAlterTableSetColumnDefault(stmt *pgq.AlterTableStmt, cmd *pgq.AlterTableCmd) (migrations.Operation, error) { def := nullable.NewNullNullable[string]() if val := cmd.GetDef().GetAConst().GetSval(); val != nil { From 2fd843dd3d0cd70c4d5f0357bd07d2438fd54335 Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Wed, 11 Dec 2024 14:06:58 +0100 Subject: [PATCH 4/6] Handle all supported constant types for default values --- pkg/sql2pgroll/alter_table.go | 18 +++++++++++++-- pkg/sql2pgroll/alter_table_test.go | 20 +++++++++++++++-- pkg/sql2pgroll/expect/alter_column.go | 32 +++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/pkg/sql2pgroll/alter_table.go b/pkg/sql2pgroll/alter_table.go index 1f6f559e..f64ed8be 100644 --- a/pkg/sql2pgroll/alter_table.go +++ b/pkg/sql2pgroll/alter_table.go @@ -4,6 +4,7 @@ package sql2pgroll import ( "fmt" + "strconv" "github.com/oapi-codegen/nullable" pgq "github.com/pganalyze/pg_query_go/v6" @@ -170,8 +171,21 @@ func convertAlterTableAddUniqueConstraint(stmt *pgq.AlterTableStmt, constraint * // to an OpAlterColumn operation. func convertAlterTableSetColumnDefault(stmt *pgq.AlterTableStmt, cmd *pgq.AlterTableCmd) (migrations.Operation, error) { def := nullable.NewNullNullable[string]() - if val := cmd.GetDef().GetAConst().GetSval(); val != nil { - def.Set(val.Sval) + if val := cmd.GetDef().GetAConst().GetVal(); val != nil { + switch v := val.(type) { + case *pgq.A_Const_Sval: + def.Set(v.Sval.GetSval()) + case *pgq.A_Const_Ival: + def.Set(strconv.FormatInt(int64(v.Ival.Ival), 10)) + case *pgq.A_Const_Fval: + def.Set(v.Fval.Fval) + case *pgq.A_Const_Boolval: + def.Set(strconv.FormatBool(v.Boolval.Boolval)) + case *pgq.A_Const_Bsval: + def.Set(v.Bsval.Bsval) + default: + return nil, fmt.Errorf("unknown constant type: %T", val) + } } return &migrations.OpAlterColumn{ Table: stmt.GetRelation().GetRelname(), diff --git a/pkg/sql2pgroll/alter_table_test.go b/pkg/sql2pgroll/alter_table_test.go index 68a4b52f..2cec4442 100644 --- a/pkg/sql2pgroll/alter_table_test.go +++ b/pkg/sql2pgroll/alter_table_test.go @@ -41,12 +41,28 @@ func TestConvertAlterTableStatements(t *testing.T) { expectedOp: expect.AlterColumnOp5, }, { - sql: "ALTER TABLE foo ALTER COLUMN bar DROP DEFAULT", + sql: "ALTER TABLE foo ALTER COLUMN bar SET DEFAULT 123", expectedOp: expect.AlterColumnOp6, }, + { + sql: "ALTER TABLE foo ALTER COLUMN bar SET DEFAULT true", + expectedOp: expect.AlterColumnOp9, + }, + { + sql: "ALTER TABLE foo ALTER COLUMN bar SET DEFAULT B'0101'", + expectedOp: expect.AlterColumnOp10, + }, + { + sql: "ALTER TABLE foo ALTER COLUMN bar SET DEFAULT 123.456", + expectedOp: expect.AlterColumnOp8, + }, + { + sql: "ALTER TABLE foo ALTER COLUMN bar DROP DEFAULT", + expectedOp: expect.AlterColumnOp7, + }, { sql: "ALTER TABLE foo ALTER COLUMN bar SET DEFAULT null", - expectedOp: expect.AlterColumnOp6, + expectedOp: expect.AlterColumnOp7, }, { sql: "ALTER TABLE foo ADD CONSTRAINT bar UNIQUE (a)", diff --git a/pkg/sql2pgroll/expect/alter_column.go b/pkg/sql2pgroll/expect/alter_column.go index 7b2af1eb..63041908 100644 --- a/pkg/sql2pgroll/expect/alter_column.go +++ b/pkg/sql2pgroll/expect/alter_column.go @@ -48,6 +48,14 @@ var AlterColumnOp5 = &migrations.OpAlterColumn{ } var AlterColumnOp6 = &migrations.OpAlterColumn{ + Table: "foo", + Column: "bar", + Default: nullable.NewNullableWithValue("123"), + Up: sql2pgroll.PlaceHolderSQL, + Down: sql2pgroll.PlaceHolderSQL, +} + +var AlterColumnOp7 = &migrations.OpAlterColumn{ Table: "foo", Column: "bar", Default: nullable.NewNullNullable[string](), @@ -55,6 +63,30 @@ var AlterColumnOp6 = &migrations.OpAlterColumn{ Down: sql2pgroll.PlaceHolderSQL, } +var AlterColumnOp8 = &migrations.OpAlterColumn{ + Table: "foo", + Column: "bar", + Default: nullable.NewNullableWithValue("123.456"), + Up: sql2pgroll.PlaceHolderSQL, + Down: sql2pgroll.PlaceHolderSQL, +} + +var AlterColumnOp9 = &migrations.OpAlterColumn{ + Table: "foo", + Column: "bar", + Default: nullable.NewNullableWithValue("true"), + Up: sql2pgroll.PlaceHolderSQL, + Down: sql2pgroll.PlaceHolderSQL, +} + +var AlterColumnOp10 = &migrations.OpAlterColumn{ + Table: "foo", + Column: "bar", + Default: nullable.NewNullableWithValue("b0101"), + Up: sql2pgroll.PlaceHolderSQL, + Down: sql2pgroll.PlaceHolderSQL, +} + func ptr[T any](v T) *T { return &v } From 6cf557e4eb7e1b38e88cd141bca0329b3ac3a07c Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Wed, 11 Dec 2024 14:24:53 +0100 Subject: [PATCH 5/6] Add more examples to docs --- pkg/sql2pgroll/alter_table.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/sql2pgroll/alter_table.go b/pkg/sql2pgroll/alter_table.go index f64ed8be..26b32c61 100644 --- a/pkg/sql2pgroll/alter_table.go +++ b/pkg/sql2pgroll/alter_table.go @@ -165,6 +165,10 @@ func convertAlterTableAddUniqueConstraint(stmt *pgq.AlterTableStmt, constraint * // convertAlterTableSetColumnDefault converts SQL statements like: // // `ALTER TABLE foo COLUMN bar SET DEFAULT 'foo'` +// `ALTER TABLE foo COLUMN bar SET DEFAULT 123` +// `ALTER TABLE foo COLUMN bar SET DEFAULT 123.456` +// `ALTER TABLE foo COLUMN bar SET DEFAULT true` +// `ALTER TABLE foo COLUMN bar SET DEFAULT B'0101'` // `ALTER TABLE foo COLUMN bar SET DEFAULT null` // `ALTER TABLE foo COLUMN bar DROP DEFAULT` // From 3db6730cbb4494f34bd4908703280171487aa99e Mon Sep 17 00:00:00 2001 From: Ryan Slade Date: Wed, 11 Dec 2024 16:40:45 +0100 Subject: [PATCH 6/6] Fall back to raw SQL for default values that are function values --- pkg/sql2pgroll/alter_table.go | 54 +++++++++++++++++++++--------- pkg/sql2pgroll/alter_table_test.go | 3 ++ 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/pkg/sql2pgroll/alter_table.go b/pkg/sql2pgroll/alter_table.go index 26b32c61..2d6b3d6b 100644 --- a/pkg/sql2pgroll/alter_table.go +++ b/pkg/sql2pgroll/alter_table.go @@ -174,30 +174,52 @@ func convertAlterTableAddUniqueConstraint(stmt *pgq.AlterTableStmt, constraint * // // to an OpAlterColumn operation. func convertAlterTableSetColumnDefault(stmt *pgq.AlterTableStmt, cmd *pgq.AlterTableCmd) (migrations.Operation, error) { - def := nullable.NewNullNullable[string]() - if val := cmd.GetDef().GetAConst().GetVal(); val != nil { - switch v := val.(type) { + operation := &migrations.OpAlterColumn{ + Table: stmt.GetRelation().GetRelname(), + Column: cmd.GetName(), + Down: PlaceHolderSQL, + Up: PlaceHolderSQL, + } + + if c := cmd.GetDef().GetAConst(); c != nil { + if c.GetIsnull() { + // The default can be set to null + operation.Default = nullable.NewNullNullable[string]() + return operation, nil + } + + // We have a constant + switch v := c.GetVal().(type) { case *pgq.A_Const_Sval: - def.Set(v.Sval.GetSval()) + operation.Default = nullable.NewNullableWithValue(v.Sval.GetSval()) case *pgq.A_Const_Ival: - def.Set(strconv.FormatInt(int64(v.Ival.Ival), 10)) + operation.Default = nullable.NewNullableWithValue(strconv.FormatInt(int64(v.Ival.Ival), 10)) case *pgq.A_Const_Fval: - def.Set(v.Fval.Fval) + operation.Default = nullable.NewNullableWithValue(v.Fval.Fval) case *pgq.A_Const_Boolval: - def.Set(strconv.FormatBool(v.Boolval.Boolval)) + operation.Default = nullable.NewNullableWithValue(strconv.FormatBool(v.Boolval.Boolval)) case *pgq.A_Const_Bsval: - def.Set(v.Bsval.Bsval) + operation.Default = nullable.NewNullableWithValue(v.Bsval.Bsval) default: - return nil, fmt.Errorf("unknown constant type: %T", val) + return nil, fmt.Errorf("unknown constant type: %T", c.GetVal()) } + + return operation, nil } - return &migrations.OpAlterColumn{ - Table: stmt.GetRelation().GetRelname(), - Column: cmd.GetName(), - Default: def, - Down: PlaceHolderSQL, - Up: PlaceHolderSQL, - }, nil + + if cmd.GetDef() != nil { + // We're setting it to something other than a constant + return nil, nil + } + + // We're not setting it to anything, which is the case when we are dropping it + if cmd.GetBehavior() == pgq.DropBehavior_DROP_RESTRICT { + operation.Default = nullable.NewNullNullable[string]() + return operation, nil + } + + // Unknown case, fall back to raw SQL + return nil, nil } func convertAlterTableDropColumn(stmt *pgq.AlterTableStmt, cmd *pgq.AlterTableCmd) (migrations.Operation, error) { diff --git a/pkg/sql2pgroll/alter_table_test.go b/pkg/sql2pgroll/alter_table_test.go index 2cec4442..65c39ba8 100644 --- a/pkg/sql2pgroll/alter_table_test.go +++ b/pkg/sql2pgroll/alter_table_test.go @@ -113,6 +113,9 @@ func TestUnconvertableAlterTableStatements(t *testing.T) { // CASCADE and IF EXISTS clauses are not represented by OpDropColumn "ALTER TABLE foo DROP COLUMN bar CASCADE", "ALTER TABLE foo DROP COLUMN IF EXISTS bar", + + // Non literal default values + "ALTER TABLE foo ALTER COLUMN bar SET DEFAULT now()", } for _, sql := range tests {