Support setting table and column comments to NULL

.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 }})'

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

docs/README.md

@@ -819,14 +819,15 @@ 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"
   }
 }
 ```
 
 * [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
 

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"
+      }
+    }
+  ]
+}

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

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=

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)
+}

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)
 		}
 	}

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,
 		})

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",

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 {

pkg/migrations/op_create_table.go

@@ -33,15 +33,15 @@ 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)
 			}
 		}
 	}
 
 	// 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)
 		}
 	}

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)

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")
+			},
+		},
 	})
 }

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"] },