fix: multiple statements from the same column#278
Merged
tianzhou merged 2 commits intopgplex:mainfrom Feb 1, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes the online rewrite logic for column SET NOT NULL operations when the same column diff also produces other statements (e.g., SET DEFAULT), resolving the duplicated work / missing default tracking described in issue #277.
Changes:
- Update
generateRewriteforDiffTypeTableColumnto only apply the NOT NULL rewrite when the current diff’s SQL actually containsSET NOT NULL, avoiding duplicate rewrite application when multiple statements exist for the same column. - Add regression fixtures under
testdata/diff/create_table/add_default_not_nullto cover a table column transitioning from nullable toNOT NULLwhile also adding a default, with the expected plan including the check-constraint-based online rewrite and a singleSET DEFAULT. - Ensure the generated plan JSON, SQL, and human-readable plan text accurately reflect the corrected behavior for this scenario.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
internal/plan/rewrite.go |
Narrows the column NOT NULL rewrite to only diffs whose SQL contains SET NOT NULL, preventing duplicate rewrite steps when a ColumnDiff emits both SET NOT NULL and SET DEFAULT. |
testdata/diff/create_table/add_default_not_null/old.sql |
Defines the original people table with a nullable created_at column for the regression scenario. |
testdata/diff/create_table/add_default_not_null/new.sql |
Defines the target schema where created_at is DEFAULT NOW() NOT NULL, exercising both default and NOT NULL changes. |
testdata/diff/create_table/add_default_not_null/diff.sql |
Captures the canonical diff SQL (SET NOT NULL then SET DEFAULT now()) that should be transformed by the rewrite logic. |
testdata/diff/create_table/add_default_not_null/plan.sql |
Asserts the exact SQL sequence for the online NOT NULL rewrite followed by setting the default, ensuring no duplicated NOT NULL workflow. |
testdata/diff/create_table/add_default_not_null/plan.txt |
Snapshot of the human-readable plan confirming a single column modification with the correct DDL sequence. |
testdata/diff/create_table/add_default_not_null/plan.json |
Machine-readable plan snapshot validating the grouped steps, operations, and paths for the corrected NOT NULL + default behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tianzhou
approved these changes
Feb 1, 2026
Contributor
tianzhou
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the contribution.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #277