diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 895821fb..9579e4aa 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -122,7 +122,7 @@ jobs: - name: Validate example migrations against JSON schema run: | - npx -y ajv-cli --spec=draft2020 validate -s schema.json -d "./examples/*.json" + npx -y ajv-cli --spec=draft2020 validate --strict-schema=false -s schema.json -d "./examples/*.json" examples: name: 'examples (pg: ${{ matrix.pgVersion }}, schema: ${{ matrix.testSchema }})' diff --git a/Makefile b/Makefile index 68644346..571240bc 100644 --- a/Makefile +++ b/Makefile @@ -5,7 +5,9 @@ generate: mv schema.json.tmp schema.json # Generate the types from the JSON schema - docker run --rm -v $$PWD/schema.json:/mnt/schema.json omissis/go-jsonschema:0.14.1 --only-models -p migrations --tags json /mnt/schema.json > pkg/migrations/types.go + # Temporarily use the `surjection/go-jsonschema` image because we need https://github.com/omissis/go-jsonschema/pull/220 + # Use the official `omissis/gojsonschema` image once 0.17.0 is released. + docker run --rm -v $$PWD/schema.json:/mnt/schema.json surjection/go-jsonschema:0.16.1 --only-models -p migrations --tags json /mnt/schema.json > pkg/migrations/types.go # Add the license header echo "// SPDX-License-Identifier: Apache-2.0" | cat - pkg/migrations/types.go > pkg/migrations/types.go.tmp diff --git a/docs/README.md b/docs/README.md index 587a082a..3e4c813e 100644 --- a/docs/README.md +++ b/docs/README.md @@ -819,7 +819,7 @@ A change comment operation changes the comment on a column. "alter_column": { "table": "table name", "column": "column name", - "comment": "new comment for column", + "comment": "new comment for column" or null, "up": "SQL expression", "down": "SQL expression" } @@ -827,6 +827,7 @@ A change comment operation changes the comment on a column. ``` * [35_alter_column_multiple.json](../examples/35_alter_column_multiple.json) +* [36_set_comment_to_null.json](../examples/36_set_comment_to_null.json) #### Add check constraint diff --git a/examples/36_set_comment_to_null.json b/examples/36_set_comment_to_null.json new file mode 100644 index 00000000..551a0102 --- /dev/null +++ b/examples/36_set_comment_to_null.json @@ -0,0 +1,14 @@ +{ + "name": "36_set_comment_to_null", + "operations": [ + { + "alter_column": { + "table": "events", + "column": "event_name", + "comment": null, + "up": "event_name", + "down": "event_name" + } + } + ] +} diff --git a/go.mod b/go.mod index e0fdab11..b7d65782 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.21 require ( github.com/google/go-cmp v0.6.0 github.com/lib/pq v1.10.9 + github.com/oapi-codegen/nullable v1.1.0 github.com/pterm/pterm v0.12.69 github.com/santhosh-tekuri/jsonschema/v5 v5.3.1 github.com/spf13/cobra v1.7.0 diff --git a/go.sum b/go.sum index 282f9ea3..08118114 100644 --- a/go.sum +++ b/go.sum @@ -225,6 +225,8 @@ github.com/moby/term v0.5.0 h1:xt8Q1nalod/v7BqbG21f8mQPqH+xAaC9C3N3wfWbVP0= github.com/moby/term v0.5.0/go.mod h1:8FzsFHVUBGZdbDsJw/ot+X+d5HLUbvklYLJ9uGfcI3Y= github.com/morikuni/aec v1.0.0 h1:nP9CBfwrvYnBRgY6qfDQkygYDmYwOilePFkwzv4dU8A= github.com/morikuni/aec v1.0.0/go.mod h1:BbKIizmSmc5MMPqRYbxO4ZU0S0+P200+tUnFx7PXmsc= +github.com/oapi-codegen/nullable v1.1.0 h1:eAh8JVc5430VtYVnq00Hrbpag9PFRGWLjxR1/3KntMs= +github.com/oapi-codegen/nullable v1.1.0/go.mod h1:KUZ3vUzkmEKY90ksAmit2+5juDIhIZhfDl+0PwOQlFY= github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U= github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM= github.com/opencontainers/image-spec v1.1.0-rc4 h1:oOxKUJWnFC4YGHCCMNql1x4YaDfYBTS5Y4x/Cgeo1E0= diff --git a/pkg/migrations/comment.go b/pkg/migrations/comment.go index a8d9f557..b3f232d3 100644 --- a/pkg/migrations/comment.go +++ b/pkg/migrations/comment.go @@ -10,19 +10,27 @@ import ( "github.com/lib/pq" ) -func addCommentToColumn(ctx context.Context, conn *sql.DB, tableName, columnName, comment string) error { +func addCommentToColumn(ctx context.Context, conn *sql.DB, tableName, columnName string, comment *string) error { _, err := conn.ExecContext(ctx, fmt.Sprintf(`COMMENT ON COLUMN %s.%s IS %s`, pq.QuoteIdentifier(tableName), pq.QuoteIdentifier(columnName), - pq.QuoteLiteral(comment))) + commentToSQL(comment))) return err } -func addCommentToTable(ctx context.Context, conn *sql.DB, tableName, comment string) error { +func addCommentToTable(ctx context.Context, conn *sql.DB, tableName string, comment *string) error { _, err := conn.ExecContext(ctx, fmt.Sprintf(`COMMENT ON TABLE %s IS %s`, pq.QuoteIdentifier(tableName), - pq.QuoteLiteral(comment))) + commentToSQL(comment))) return err } + +func commentToSQL(comment *string) string { + if comment == nil { + return "NULL" + } + + return pq.QuoteLiteral(*comment) +} diff --git a/pkg/migrations/op_add_column.go b/pkg/migrations/op_add_column.go index 09d81699..aa251e26 100644 --- a/pkg/migrations/op_add_column.go +++ b/pkg/migrations/op_add_column.go @@ -23,7 +23,7 @@ func (o *OpAddColumn) Start(ctx context.Context, conn *sql.DB, stateSchema strin } if o.Column.Comment != nil { - if err := addCommentToColumn(ctx, conn, o.Table, TemporaryName(o.Column.Name), *o.Column.Comment); err != nil { + if err := addCommentToColumn(ctx, conn, o.Table, TemporaryName(o.Column.Name), o.Column.Comment); err != nil { return nil, fmt.Errorf("failed to add comment to column: %w", err) } } diff --git a/pkg/migrations/op_alter_column.go b/pkg/migrations/op_alter_column.go index 3bffe320..8653fad6 100644 --- a/pkg/migrations/op_alter_column.go +++ b/pkg/migrations/op_alter_column.go @@ -296,11 +296,16 @@ func (o *OpAlterColumn) subOperations() []Operation { Down: o.Down, }) } - if o.Comment != nil { + if o.Comment.IsSpecified() { + var comment *string + if c, err := o.Comment.Get(); err == nil { + comment = &c + } + ops = append(ops, &OpSetComment{ Table: o.Table, Column: o.Column, - Comment: *o.Comment, + Comment: comment, Up: o.Up, Down: o.Down, }) diff --git a/pkg/migrations/op_alter_column_test.go b/pkg/migrations/op_alter_column_test.go index a5671bb6..6d286340 100644 --- a/pkg/migrations/op_alter_column_test.go +++ b/pkg/migrations/op_alter_column_test.go @@ -6,6 +6,7 @@ import ( "database/sql" "testing" + "github.com/oapi-codegen/nullable" "github.com/stretchr/testify/assert" "github.com/xataio/pgroll/pkg/migrations" "github.com/xataio/pgroll/pkg/testutils" @@ -48,7 +49,7 @@ func TestAlterColumnMultipleSubOperations(t *testing.T) { Down: "name", Name: ptr("event_name"), Type: ptr("text"), - Comment: ptr("the name of the event"), + Comment: nullable.NewNullableWithValue("the name of the event"), Nullable: ptr(false), Check: &migrations.CheckConstraint{ Name: "event_name_length", diff --git a/pkg/migrations/op_common_test.go b/pkg/migrations/op_common_test.go index 76fe0c58..c0d75d8b 100644 --- a/pkg/migrations/op_common_test.go +++ b/pkg/migrations/op_common_test.go @@ -166,11 +166,18 @@ func ColumnMustHaveType(t *testing.T, db *sql.DB, schema, table, column, expecte func ColumnMustHaveComment(t *testing.T, db *sql.DB, schema, table, column, expectedComment string) { t.Helper() - if !columnHasComment(t, db, schema, table, column, expectedComment) { + if !columnHasComment(t, db, schema, table, column, &expectedComment) { t.Fatalf("Expected column %q to have comment %q", column, expectedComment) } } +func ColumnMustNotHaveComment(t *testing.T, db *sql.DB, schema, table, column string) { + t.Helper() + if !columnHasComment(t, db, schema, table, column, nil) { + t.Fatalf("Expected column %q to have no comment", column) + } +} + func TableMustHaveComment(t *testing.T, db *sql.DB, schema, table, expectedComment string) { t.Helper() if !tableHasComment(t, db, schema, table, expectedComment) { @@ -497,7 +504,7 @@ func columnHasType(t *testing.T, db *sql.DB, schema, table, column, expectedType return expectedType == actualType } -func columnHasComment(t *testing.T, db *sql.DB, schema, table, column, expectedComment string) bool { +func columnHasComment(t *testing.T, db *sql.DB, schema, table, column string, expectedComment *string) bool { t.Helper() var actualComment *string @@ -513,7 +520,10 @@ func columnHasComment(t *testing.T, db *sql.DB, schema, table, column, expectedC t.Fatal(err) } - return actualComment != nil && *actualComment == expectedComment + if expectedComment == nil { + return actualComment == nil + } + return actualComment != nil && *expectedComment == *actualComment } func tableHasComment(t *testing.T, db *sql.DB, schema, table, expectedComment string) bool { diff --git a/pkg/migrations/op_create_table.go b/pkg/migrations/op_create_table.go index 93b8c7ec..adc4a371 100644 --- a/pkg/migrations/op_create_table.go +++ b/pkg/migrations/op_create_table.go @@ -33,7 +33,7 @@ func (o *OpCreateTable) Start(ctx context.Context, conn *sql.DB, stateSchema str // Add comments to any columns that have them for _, col := range o.Columns { if col.Comment != nil { - if err := addCommentToColumn(ctx, conn, tempName, col.Name, *col.Comment); err != nil { + if err := addCommentToColumn(ctx, conn, tempName, col.Name, col.Comment); err != nil { return nil, fmt.Errorf("failed to add comment to column: %w", err) } } @@ -41,7 +41,7 @@ func (o *OpCreateTable) Start(ctx context.Context, conn *sql.DB, stateSchema str // Add comment to the table itself if o.Comment != nil { - if err := addCommentToTable(ctx, conn, tempName, *o.Comment); err != nil { + if err := addCommentToTable(ctx, conn, tempName, o.Comment); err != nil { return nil, fmt.Errorf("failed to add comment to table: %w", err) } } diff --git a/pkg/migrations/op_set_comment.go b/pkg/migrations/op_set_comment.go index 6adf93c5..fc8d13aa 100644 --- a/pkg/migrations/op_set_comment.go +++ b/pkg/migrations/op_set_comment.go @@ -10,11 +10,11 @@ import ( ) type OpSetComment struct { - Table string `json:"table"` - Column string `json:"column"` - Comment string `json:"comment"` - Up string `json:"up"` - Down string `json:"down"` + Table string `json:"table"` + Column string `json:"column"` + Comment *string `json:"comment"` + Up string `json:"up"` + Down string `json:"down"` } var _ Operation = (*OpSetComment)(nil) diff --git a/pkg/migrations/op_set_comment_test.go b/pkg/migrations/op_set_comment_test.go index 56b3d8ed..683120ed 100644 --- a/pkg/migrations/op_set_comment_test.go +++ b/pkg/migrations/op_set_comment_test.go @@ -6,6 +6,7 @@ import ( "database/sql" "testing" + "github.com/oapi-codegen/nullable" "github.com/stretchr/testify/assert" "github.com/xataio/pgroll/pkg/migrations" ) @@ -43,7 +44,7 @@ func TestSetComment(t *testing.T) { &migrations.OpAlterColumn{ Table: "users", Column: "name", - Comment: ptr("name of the user"), + Comment: nullable.NewNullableWithValue("name of the user"), }, }, }, @@ -124,7 +125,7 @@ func TestSetComment(t *testing.T) { &migrations.OpAlterColumn{ Table: "users", Column: "name", - Comment: ptr("name of the user"), + Comment: nullable.NewNullableWithValue("name of the user"), Up: "'rewritten by up SQL'", Down: "'rewritten by down SQL'", }, @@ -178,5 +179,101 @@ func TestSetComment(t *testing.T) { }, rows) }, }, + { + name: "set column comment to NULL", + migrations: []migrations.Migration{ + { + Name: "01_add_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "name", + Type: "text", + Comment: ptr("apples"), + }, + }, + }, + }, + }, + { + Name: "02_set_comment", + Operations: migrations.Operations{ + &migrations.OpAlterColumn{ + Table: "users", + Column: "name", + Comment: nullable.NewNullNullable[string](), + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // The old column should have the old comment. + ColumnMustHaveComment(t, db, schema, "users", "name", "apples") + + // The new column should have no comment. + ColumnMustNotHaveComment(t, db, schema, "users", migrations.TemporaryName("name")) + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + // The old column should have the old comment. + ColumnMustHaveComment(t, db, schema, "users", "name", "apples") + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // The new column should have no comment. + ColumnMustNotHaveComment(t, db, schema, "users", "name") + }, + }, + { + name: "leaving the comment unspecified does not change the comment", + migrations: []migrations.Migration{ + { + Name: "01_add_table", + Operations: migrations.Operations{ + &migrations.OpCreateTable{ + Name: "users", + Columns: []migrations.Column{ + { + Name: "id", + Type: "serial", + Pk: ptr(true), + }, + { + Name: "name", + Type: "text", + Comment: ptr("apples"), + }, + }, + }, + }, + }, + { + Name: "02_set_comment", + Operations: migrations.Operations{ + &migrations.OpAlterColumn{ + Table: "users", + Column: "name", + Nullable: ptr(true), + Down: "(SELECT CASE WHEN name IS NULL THEN 'placeholder' ELSE name END)", + }, + }, + }, + }, + afterStart: func(t *testing.T, db *sql.DB, schema string) { + // The comment has not been changed on the temporary column. + ColumnMustHaveComment(t, db, schema, "users", migrations.TemporaryName("name"), "apples") + }, + afterRollback: func(t *testing.T, db *sql.DB, schema string) { + }, + afterComplete: func(t *testing.T, db *sql.DB, schema string) { + // The comment has not been changed on the new column. + ColumnMustHaveComment(t, db, schema, "users", "name", "apples") + }, + }, }) } diff --git a/pkg/migrations/types.go b/pkg/migrations/types.go index 56f43038..e13aa7ff 100644 --- a/pkg/migrations/types.go +++ b/pkg/migrations/types.go @@ -3,6 +3,8 @@ package migrations +import "github.com/oapi-codegen/nullable" + // Check constraint definition type CheckConstraint struct { // Constraint expression @@ -86,7 +88,7 @@ type OpAlterColumn struct { Column string `json:"column"` // New comment on the column - Comment *string `json:"comment,omitempty"` + Comment nullable.Nullable[string] `json:"comment,omitempty"` // Default value of the column Default *string `json:"default,omitempty"` diff --git a/schema.json b/schema.json index 0b333e92..44ae5e90 100644 --- a/schema.json +++ b/schema.json @@ -159,7 +159,12 @@ }, "comment": { "description": "New comment on the column", - "type": "string" + "type": ["string", "null"], + "goJSONSchema": { + "imports": ["github.com/oapi-codegen/nullable"], + "nillable": true, + "type": "nullable.Nullable[string]" + } }, "up": { "default": "", @@ -174,6 +179,8 @@ { "required": ["check"] }, { "required": ["type"] }, { "required": ["nullable"] }, + { "required": ["default"] }, + { "required": ["comment"] }, { "required": ["unique"] }, { "required": ["references"] } ], @@ -186,6 +193,8 @@ { "required": ["check"] }, { "required": ["type"] }, { "required": ["nullable"] }, + { "required": ["default"] }, + { "required": ["comment"] }, { "required": ["unique"] }, { "required": ["references"] }, { "required": ["up"] },