Preserve comments on column duplication (#323)

Ensure that column comments are preserved when a column is duplicated.

Part of #227

pkg/migrations/duplicate.go

@@ -58,6 +58,7 @@ func (d *Duplicator) Duplicate(ctx context.Context) error {
 		cAddForeignKeySQL      = `ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s) ON DELETE %s`
 		cAddCheckConstraintSQL = `ADD CONSTRAINT %s %s NOT VALID`
 		cCreateUniqueIndexSQL  = `CREATE UNIQUE INDEX CONCURRENTLY %s ON %s (%s)`
+		cCommentOnColumnSQL    = `COMMENT ON COLUMN %s.%s IS %s`
 	)
 
 	// Generate SQL to duplicate the column's name and type
@@ -116,6 +117,20 @@ func (d *Duplicator) Duplicate(ctx context.Context) error {
 		return err
 	}
 
+	// Generate SQL to duplicate the column's comment
+	if d.column.Comment != "" {
+		sql = fmt.Sprintf(cCommentOnColumnSQL,
+			pq.QuoteIdentifier(d.table.Name),
+			pq.QuoteIdentifier(d.asName),
+			pq.QuoteLiteral(d.column.Comment),
+		)
+
+		_, err = d.conn.ExecContext(ctx, sql)
+		if err != nil {
+			return err
+		}
+	}
+
 	// Generate SQL to duplicate any unique constraints on the column
 	// The constraint is duplicated by adding a unique index on the column concurrently.
 	// The index is converted into a unique constraint on migration completion.

pkg/migrations/op_change_type_test.go

@@ -472,6 +472,53 @@ func TestChangeColumnType(t *testing.T) {
 				})
 			},
 		},
+		{
+			name: "changing column type preserves any comments on the column",
+			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:    "username",
+									Type:    "text",
+									Comment: ptr("the name of the user"),
+								},
+							},
+						},
+					},
+				},
+				{
+					Name: "02_change_type",
+					Operations: migrations.Operations{
+						&migrations.OpAlterColumn{
+							Table:  "users",
+							Column: "username",
+							Type:   ptr("varchar(255)"),
+							Up:     ptr("username"),
+							Down:   ptr("username"),
+						},
+					},
+				},
+			},
+			afterStart: func(t *testing.T, db *sql.DB, schema string) {
+				// The duplicated column has a comment defined on it
+				ColumnMustHaveComment(t, db, schema, "users", migrations.TemporaryName("username"), "the name of the user")
+			},
+			afterRollback: func(t *testing.T, db *sql.DB, schema string) {
+			},
+			afterComplete: func(t *testing.T, db *sql.DB, schema string) {
+				// The final column has a comment defined on it
+				ColumnMustHaveComment(t, db, schema, "users", "username", "the name of the user")
+			},
+		},
 	})
 }
 

pkg/migrations/op_common_test.go

@@ -484,7 +484,7 @@ func columnHasType(t *testing.T, db *sql.DB, schema, table, column, expectedType
 func columnHasComment(t *testing.T, db *sql.DB, schema, table, column, expectedComment string) bool {
 	t.Helper()
 
-	var actualComment string
+	var actualComment *string
 	err := db.QueryRow(fmt.Sprintf(`
     SELECT col_description(
       %[1]s::regclass,
@@ -497,7 +497,7 @@ func columnHasComment(t *testing.T, db *sql.DB, schema, table, column, expectedC
 		t.Fatal(err)
 	}
 
-	return expectedComment == actualComment
+	return actualComment != nil && *actualComment == expectedComment
 }
 
 func tableHasComment(t *testing.T, db *sql.DB, schema, table, expectedComment string) bool {

pkg/migrations/op_drop_constraint_test.go

@@ -815,6 +815,55 @@ func TestDropConstraint(t *testing.T) {
 				}, testutils.NotNullViolationErrorCode)
 			},
 		},
+		{
+			name: "dropping a unique constraint preserves any comment on the column",
+			migrations: []migrations.Migration{
+				{
+					Name: "01_add_table",
+					Operations: migrations.Operations{
+						&migrations.OpCreateTable{
+							Name: "posts",
+							Columns: []migrations.Column{
+								{
+									Name: "id",
+									Type: "serial",
+									Pk:   ptr(true),
+								},
+								{
+									Name:     "title",
+									Type:     "text",
+									Unique:   ptr(true),
+									Nullable: ptr(false),
+									Comment:  ptr("the title of the post"),
+								},
+							},
+						},
+					},
+				},
+				{
+					Name: "02_drop_unique_constraint",
+					Operations: migrations.Operations{
+						&migrations.OpDropConstraint{
+							Table:  "posts",
+							Column: "title",
+							Name:   "_pgroll_new_posts_title_key",
+							Up:     "title",
+							Down:   "title",
+						},
+					},
+				},
+			},
+			afterStart: func(t *testing.T, db *sql.DB, schema string) {
+				// The duplicated column has a comment defined on it
+				ColumnMustHaveComment(t, db, schema, "posts", migrations.TemporaryName("title"), "the title of the post")
+			},
+			afterRollback: func(t *testing.T, db *sql.DB, schema string) {
+			},
+			afterComplete: func(t *testing.T, db *sql.DB, schema string) {
+				// The final column has a comment defined on it
+				ColumnMustHaveComment(t, db, schema, "posts", "title", "the title of the post")
+			},
+		},
 	})
 }
 

pkg/migrations/op_drop_not_null_test.go

@@ -492,6 +492,54 @@ func TestDropNotNull(t *testing.T) {
 				})
 			},
 		},
+		{
+			name: "dropping not null retains any comments defined on the column",
+			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",
+									Nullable: ptr(false),
+									Comment:  ptr("the name of the user"),
+								},
+							},
+						},
+					},
+				},
+				{
+					Name: "02_set_not_null",
+					Operations: migrations.Operations{
+						&migrations.OpAlterColumn{
+							Table:    "users",
+							Column:   "name",
+							Nullable: ptr(true),
+							Up:       ptr("name"),
+							Down:     ptr("(SELECT CASE WHEN name IS NULL THEN 'anonymous' ELSE name END)"),
+						},
+					},
+				},
+			},
+			afterStart: func(t *testing.T, db *sql.DB, schema string) {
+				// The duplicated column has a comment defined on it
+				ColumnMustHaveComment(t, db, schema, "users", migrations.TemporaryName("name"), "the name of the user")
+			},
+			afterRollback: func(t *testing.T, db *sql.DB, schema string) {
+			},
+			afterComplete: func(t *testing.T, db *sql.DB, schema string) {
+				// The final column has a comment defined on it
+				ColumnMustHaveComment(t, db, schema, "users", "name", "the name of the user")
+			},
+		},
 	})
 }
 

pkg/migrations/op_set_check_test.go

@@ -492,6 +492,56 @@ func TestSetCheckConstraint(t *testing.T) {
 				})
 			},
 		},
+		{
+			name: "comments are preserved when adding a check constraint",
+			migrations: []migrations.Migration{
+				{
+					Name: "01_add_table",
+					Operations: migrations.Operations{
+						&migrations.OpCreateTable{
+							Name: "posts",
+							Columns: []migrations.Column{
+								{
+									Name: "id",
+									Type: "serial",
+									Pk:   ptr(true),
+								},
+								{
+									Name:    "title",
+									Type:    "text",
+									Comment: ptr("the title of the post"),
+								},
+							},
+						},
+					},
+				},
+				{
+					Name: "02_add_check_constraint",
+					Operations: migrations.Operations{
+						&migrations.OpAlterColumn{
+							Table:  "posts",
+							Column: "title",
+							Check: &migrations.CheckConstraint{
+								Name:       "check_title_length",
+								Constraint: "length(title) > 3",
+							},
+							Up:   ptr("(SELECT CASE WHEN length(title) <= 3 THEN LPAD(title, 4, '-') ELSE title END)"),
+							Down: ptr("title"),
+						},
+					},
+				},
+			},
+			afterStart: func(t *testing.T, db *sql.DB, schema string) {
+				// The duplicated column has a comment defined on it
+				ColumnMustHaveComment(t, db, schema, "posts", migrations.TemporaryName("title"), "the title of the post")
+			},
+			afterRollback: func(t *testing.T, db *sql.DB, schema string) {
+			},
+			afterComplete: func(t *testing.T, db *sql.DB, schema string) {
+				// The final column has a comment defined on it
+				ColumnMustHaveComment(t, db, schema, "posts", "title", "the title of the post")
+			},
+		},
 	})
 }
 

pkg/migrations/op_set_fk_test.go

@@ -1050,6 +1050,75 @@ func TestSetForeignKey(t *testing.T) {
 				})
 			},
 		},
+		{
+			name: "comments are preserved when adding a foreign key constraint",
+			migrations: []migrations.Migration{
+				{
+					Name: "01_add_tables",
+					Operations: migrations.Operations{
+						&migrations.OpCreateTable{
+							Name: "users",
+							Columns: []migrations.Column{
+								{
+									Name: "id",
+									Type: "serial",
+									Pk:   ptr(true),
+								},
+								{
+									Name: "name",
+									Type: "text",
+								},
+							},
+						},
+						&migrations.OpCreateTable{
+							Name: "posts",
+							Columns: []migrations.Column{
+								{
+									Name: "id",
+									Type: "serial",
+									Pk:   ptr(true),
+								},
+								{
+									Name: "title",
+									Type: "text",
+								},
+								{
+									Name:    "user_id",
+									Type:    "integer",
+									Comment: ptr("the id of the author"),
+								},
+							},
+						},
+					},
+				},
+				{
+					Name: "02_add_fk_constraint",
+					Operations: migrations.Operations{
+						&migrations.OpAlterColumn{
+							Table:  "posts",
+							Column: "user_id",
+							References: &migrations.ForeignKeyReference{
+								Name:   "fk_users_id",
+								Table:  "users",
+								Column: "id",
+							},
+							Up:   ptr("(SELECT CASE WHEN EXISTS (SELECT 1 FROM users WHERE users.id = user_id) THEN user_id ELSE NULL END)"),
+							Down: ptr("user_id"),
+						},
+					},
+				},
+			},
+			afterStart: func(t *testing.T, db *sql.DB, schema string) {
+				// The duplicated column has a comment defined on it
+				ColumnMustHaveComment(t, db, schema, "posts", migrations.TemporaryName("user_id"), "the id of the author")
+			},
+			afterRollback: func(t *testing.T, db *sql.DB, schema string) {
+			},
+			afterComplete: func(t *testing.T, db *sql.DB, schema string) {
+				// The final column has a comment defined on it
+				ColumnMustHaveComment(t, db, schema, "posts", "user_id", "the id of the author")
+			},
+		},
 	})
 }
 

pkg/migrations/op_set_notnull_test.go

@@ -496,6 +496,53 @@ func TestSetNotNull(t *testing.T) {
 				})
 			},
 		},
+		{
+			name: "setting a column to not null retains any comment defined on the column",
+			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",
+									Nullable: ptr(true),
+									Comment:  ptr("the name of the user"),
+								},
+							},
+						},
+					},
+				},
+				{
+					Name: "02_set_not_null",
+					Operations: migrations.Operations{
+						&migrations.OpAlterColumn{
+							Table:    "users",
+							Column:   "name",
+							Nullable: ptr(false),
+							Up:       ptr("(SELECT CASE WHEN name IS NULL THEN 'anonymous' ELSE name END)"),
+						},
+					},
+				},
+			},
+			afterStart: func(t *testing.T, db *sql.DB, schema string) {
+				// The duplicated column has a comment defined on it
+				ColumnMustHaveComment(t, db, schema, "users", migrations.TemporaryName("name"), "the name of the user")
+			},
+			afterRollback: func(t *testing.T, db *sql.DB, schema string) {
+			},
+			afterComplete: func(t *testing.T, db *sql.DB, schema string) {
+				// The final column has a comment defined on it
+				ColumnMustHaveComment(t, db, schema, "users", "name", "the name of the user")
+			},
+		},
 	})
 }