Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql2pgroll: Support set and drop column defaults #526

Merged
merged 6 commits into from
Dec 11, 2024
Merged

sql2pgroll: Support set and drop column defaults #526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
da2efe4
sql2pgroll: Support set and drop column defaults
ryanslade Dec 11, 2024
1834fc4
Use backticks
ryanslade Dec 11, 2024
89ce4f7
Fix comment
ryanslade Dec 11, 2024
2fd843d
Handle all supported constant types for default values
ryanslade Dec 11, 2024
6cf557e
Add more examples to docs
ryanslade Dec 11, 2024
3db6730
Fall back to raw SQL for default values that are function values
ryanslade Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 62 additions & 3 deletions pkg/sql2pgroll/alter_table.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ package sql2pgroll

import (
"fmt"
"strconv"

"github.com/oapi-codegen/nullable"
pgq "github.com/pganalyze/pg_query_go/v6"

"github.com/xataio/pgroll/pkg/migrations"
Expand Down Expand Up @@ -38,6 +40,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 {
Expand Down Expand Up @@ -158,11 +162,66 @@ 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 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`
//
// to an OpDropColumn operation.
// to an OpAlterColumn operation.
func convertAlterTableSetColumnDefault(stmt *pgq.AlterTableStmt, cmd *pgq.AlterTableCmd) (migrations.Operation, error) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since OpAlterColumn only supports nullable string defaults, this will fail for any other kind of default for example non strings or the value of a function call.

In those cases should we fall back to raw sql?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like if the new default value is a constant, we can support these types:

type A_Const struct {
	state         protoimpl.MessageState
	sizeCache     protoimpl.SizeCache
	unknownFields protoimpl.UnknownFields

	// Types that are assignable to Val:
	//
	//	*A_Const_Ival
	//	*A_Const_Fval
	//	*A_Const_Boolval
	//	*A_Const_Sval
	//	*A_Const_Bsval
	Val      isA_Const_Val `protobuf_oneof:"val"`
	Isnull   bool          `protobuf:"varint,10,opt,name=isnull,proto3" json:"isnull,omitempty"`
	Location int32         `protobuf:"varint,11,opt,name=location,proto3" json:"location,omitempty"`
}

And then convert them to a string

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done this

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:
operation.Default = nullable.NewNullableWithValue(v.Sval.GetSval())
case *pgq.A_Const_Ival:
operation.Default = nullable.NewNullableWithValue(strconv.FormatInt(int64(v.Ival.Ival), 10))
case *pgq.A_Const_Fval:
operation.Default = nullable.NewNullableWithValue(v.Fval.Fval)
case *pgq.A_Const_Boolval:
operation.Default = nullable.NewNullableWithValue(strconv.FormatBool(v.Boolval.Boolval))
case *pgq.A_Const_Bsval:
operation.Default = nullable.NewNullableWithValue(v.Bsval.Bsval)
default:
return nil, fmt.Errorf("unknown constant type: %T", c.GetVal())
}

return operation, 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) {
if !canConvertDropColumn(cmd) {
return nil, nil
Expand Down
31 changes: 31 additions & 0 deletions pkg/sql2pgroll/alter_table_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,34 @@ 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 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.AlterColumnOp7,
},
{
sql: "ALTER TABLE foo ADD CONSTRAINT bar UNIQUE (a)",
expectedOp: expect.CreateConstraintOp1,
Expand Down Expand Up @@ -85,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 {
Expand Down
50 changes: 50 additions & 0 deletions pkg/sql2pgroll/expect/alter_column.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
package expect

import (
"github.com/oapi-codegen/nullable"

"github.com/xataio/pgroll/pkg/migrations"
"github.com/xataio/pgroll/pkg/sql2pgroll"
)
Expand Down Expand Up @@ -37,6 +39,54 @@ 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.NewNullableWithValue("123"),
Up: sql2pgroll.PlaceHolderSQL,
Down: sql2pgroll.PlaceHolderSQL,
}

var AlterColumnOp7 = &migrations.OpAlterColumn{
Table: "foo",
Column: "bar",
Default: nullable.NewNullNullable[string](),
Up: sql2pgroll.PlaceHolderSQL,
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
}