Skip to content

Comments

fix: include column comments when adding new columns#242

Merged
tianzhou merged 2 commits intopgplex:mainfrom
asonawalla:fix/add-column-comment-single-pass
Jan 15, 2026
Merged

fix: include column comments when adding new columns#242
tianzhou merged 2 commits intopgplex:mainfrom
asonawalla:fix/add-column-comment-single-pass

Conversation

@asonawalla
Copy link
Contributor

Summary

When adding a new column with a COMMENT ON COLUMN to an existing table, the comment was not included in the first plan - it required a second plan/apply cycle to converge.

This adds a loop after the AddedColumns processing in generateAlterTableStatements to emit COMMENT ON COLUMN statements for any added columns that have comments. This follows the same pattern already used for new tables (lines 437-453) and new indexes (index.go lines 279-282).

Related discussion: #240

Test plan

  • Extended testdata/diff/comment/mixed_comments/ test to include a new column with a comment
  • Verified all 10 comment tests pass for both TestDiffFromFiles and TestPlanAndApply

When adding a new column with a COMMENT ON COLUMN to an existing table,
the comment was not included in the first plan. It required a second
plan/apply cycle to converge.

Added a loop after the AddedColumns processing in generateAlterTableStatements
to emit COMMENT ON COLUMN statements for any added columns that have comments.
This follows the same pattern used for new tables and new indexes.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug where adding a new column with a comment to an existing table required two plan/apply cycles to converge. The fix adds a loop to emit COMMENT ON COLUMN statements for newly added columns that have comments, following the same pattern already used for new tables and new indexes.

Changes:

  • Added comment generation for newly added columns in generateAlterTableStatements
  • Extended test case to verify the fix works correctly

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/diff/table.go Added loop (lines 769-784) to generate COMMENT ON COLUMN statements for newly added columns with comments
testdata/diff/comment/mixed_comments/new.sql Added new views column with comment to test case
testdata/diff/comment/mixed_comments/diff.sql Expected diff output showing column addition with comment
testdata/diff/comment/mixed_comments/plan.sql Expected plan SQL with both column and comment statements
testdata/diff/comment/mixed_comments/plan.txt Expected plan text showing column and comment as separate operations
testdata/diff/comment/mixed_comments/plan.json Expected plan JSON with both column creation and comment creation entries

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

@tianzhou tianzhou left a comment

Choose a reason for hiding this comment

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

LGTM

@tianzhou tianzhou merged commit 424b49a into pgplex:main Jan 15, 2026
1 check passed
alecthomas pushed a commit to alecthomas/pgschema that referenced this pull request Jan 26, 2026
* fix: include column comments when adding new columns

When adding a new column with a COMMENT ON COLUMN to an existing table,
the comment was not included in the first plan. It required a second
plan/apply cycle to converge.

Added a loop after the AddedColumns processing in generateAlterTableStatements
to emit COMMENT ON COLUMN statements for any added columns that have comments.
This follows the same pattern used for new tables and new indexes.

* Update internal/diff/table.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Tianzhou <t@bytebase.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants