feat(linter): add default rollback compatibility rules - #1383
Merged
Merged
Conversation
馃懛 Deploy request for squawkhq pending review.Visit the deploys page to approve it
|
t-monaghan
marked this pull request as draft
October 5, 2026 05:00
t-monaghan
force-pushed
the
compat-default-split
branch
5 times, most recently
from
October 5, 2026 05:57
c2188e3 to
9cc9682
Compare
t-monaghan
force-pushed
the
compat-default-split
branch
from
October 5, 2026 06:07
9cc9682 to
5cb8256
Compare
t-monaghan
marked this pull request as ready for review
October 5, 2026 06:08
sbdchd
reviewed
Oct 5, 2026
| if let ast::Stmt::DropDomain(node) = stmt { | ||
| ctx.report(Violation::for_node( | ||
| Rule::BanDropDomain, | ||
| "Dropping a domain may break existing clients.".into(), |
Owner
There was a problem hiding this comment.
just out of curiosity, are you using domains? I've heard they're kinda prickly to use in practice
Contributor
Author
There was a problem hiding this comment.
We're not, but given the breadth of our services I wouldn't make the assumption that we never will. I'm happy to be guided on this being opt-in if it seems like overkill
Owner
There was a problem hiding this comment.
Gotcha yeah I think it's fine to lean on by default
sbdchd
reviewed
Oct 5, 2026
| "Dropping a table may break existing clients.".into(), | ||
| drop_table.syntax(), | ||
| )); | ||
| } else if let ast::Stmt::DropForeignTable(drop_table) = stmt { |
Contributor
Author
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
2 participants
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.
Purpose
At Culture Amp the SRE team are proposing using Squawk to review SQL migrations and to increase confidence that we can roll back our applications. As we adopt blue/green deployments, this check becomes more important: an earlier application revision may need to run against a database that has already been migrated.
Rolling back the application does not roll back the database migration or remove data written by the newer revision. The earlier revision must still be able to read and write using the migrated database.
These default rules flag migration operations that directly change or remove database contracts used by earlier application revisions. They help identify risks before deployment, alongside testing earlier revisions against the migrated database.
Context
This PR adds nine rules enabled by default:
ban-drop-schema,ban-drop-sequence,ban-drop-domaindetectDROP SCHEMA,DROP SEQUENCE, andDROP DOMAIN. Queries that reference a dropped object fail.ban-drop-constraintdetectsDROP CONSTRAINT. Dropping a unique constraint can make an earlier application'sINSERT ... ON CONFLICTfail.ban-drop-generated-expressiondetectsDROP EXPRESSION, which changes a generated column into an ordinary column.ban-drop-extensiondetectsDROP EXTENSION, which can remove functions, types, and operators used by the earlier application.ban-alter-identitydetects identity column additions, changes, and removals. ChangingidtoGENERATED ALWAYSrejects an earlier application'sINSERT INTO t (id) VALUES (1)unless it usesOVERRIDING SYSTEM VALUE. An application that omitsidmay continue to work.renaming-objectdetects renames of views, sequences, types, enum values, foreign tables, routines, aggregates, roles, databases, triggers, policies, and constraints.ban-set-schemadetectsSET SCHEMAon tables, foreign tables, views, types, sequences, functions, procedures, routines, aggregates, and extensions.This PR also extends nine existing rules:
adding-not-nullable-field,ban-drop-default, andban-drop-not-nullalso check domains and foreign tables.ban-drop-defaultalso checks view columns.ban-drop-column,changing-column-type, andrenaming-columnalso check foreign tables and composite type attributes.renaming-columnalso checks views and materialized views.ban-drop-functionalso checksDROP AGGREGATEandDROP ROUTINE.ban-drop-tablealso checksDROP FOREIGN TABLE.ban-drop-typealso checksDROP OPERATOR,DROP OPERATOR CLASS,DROP OPERATOR FAMILY, andDROP CAST.The PR includes tests, diagnostics, rule documentation, and a changelog entry.