Skip to content

Add linter for rename column#617

Merged
morgo merged 3 commits intoblock:mainfrom
morgo:mtocker-lint-rename
Feb 25, 2026
Merged

Add linter for rename column#617
morgo merged 3 commits intoblock:mainfrom
morgo:mtocker-lint-rename

Conversation

@morgo
Copy link
Collaborator

@morgo morgo commented Feb 25, 2026

A Pull Request should be associated with an Issue.

We wish to have discussions in Issues. A single issue may be targeted by multiple PRs.
If you're offering a new feature or fixing anything, we'd like to know beforehand in Issues,
and potentially we'll be able to point development in a particular direction.
Further notes in https://github.com/block/spirit/blob/main/.github/CONTRIBUTING.md

Part of #446

Example output:

mtocker@BLKD5234QXY40 bin % ./spirit diff --source-dsn="spirit:spirit@tcp(127.0.0.1:3306)/source"  --target-alter="alter table t1_old rename column val to val2"
-- [WARNING] rename_column: Column rename detected in table "t1_old": "val" to "val2". Renaming a column cannot be done atomically across application pods, and ORMs that generate column names at compile time (e.g. jOOQ) will break until code is recompiled (Table: t1_old, Column: val) Suggestion: Use ADD COLUMN + DROP COLUMN instead of RENAME COLUMN. This is the only safe approach

alter table t1_old rename column val to val2;

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

Adds a new native lint rule to detect column renames in ALTER TABLE statements, aligning with the broader move toward parser-based linting described in issue #446.

Changes:

  • Add rename_column linter to warn on RENAME COLUMN and CHANGE COLUMN old new ... rename patterns.
  • Add unit tests covering rename and non-rename ALTER variants.
  • Refactor violation sorting/printing helpers into violation.go (and remove the old cmd_common.go), plus ignore bin/ in .gitignore.

Reviewed changes

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

Show a summary per file
File Description
pkg/lint/lint_rename_column.go Implements the new rename_column linter using TiDB AST specs.
pkg/lint/lint_rename_column_test.go Adds test coverage for rename detection across ALTER variants.
pkg/lint/violation.go Centralizes violation sorting/printing helpers used by CLI output.
pkg/lint/cmd_common.go Removed after moving shared helper functions into violation.go.
.gitignore Ignores bin/ output directory.

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

Column: strPtr(oldName),
},
Message: fmt.Sprintf("Column rename detected in table %q: %q to %q. Renaming a column cannot be done atomically across application pods, and ORMs that generate column names at compile time (e.g. jOOQ) will break until code is recompiled", change.Table, oldName, newName),
Severity: SeverityWarning,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this be a higher severity?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For now most things are warnings. We will have to revisit all the severity levels though. I'd like to have a 1 pager which explains them and why.

@morgo morgo requested a review from aparajon February 25, 2026 17:41
@morgo morgo enabled auto-merge February 25, 2026 19:27
@morgo morgo merged commit 0e06afd into block:main Feb 25, 2026
9 of 12 checks passed
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.

3 participants