The down half of a migration is not applied: golang-migrate down files and goose, sql-migrate and dbmate down sections - #17
Merged
Conversation
…s and goose, sql-migrate and dbmate down sections
cdclint applied every *.sql file in name order, and every statement in
each file. A forward migrate never runs a migration's down half, so the
schema cdclint read could differ from the one the connector sees:
- goose, sql-migrate and dbmate keep both halves in one file, the down
section after the up one. Applied whole, each migration was undone
right after it ran: a goose table was created and dropped again, and
every sink reading it got "no table produces this topic".
- golang-migrate keeps the down half in {version}_{title}.down.sql. It
sorts just before its own up file, so it is usually a no-op, but not
always: Mattermost v10.11.0's 000092 down file drops CreateAt from
Reactions instead of TeamMembers, and reading it deleted a real column.
Measured on Mattermost's 140 MySQL migrations: 1 of 71 tables differed.
A new internal/migrate package names both: Down(path) for golang-migrate
files, which ReadFiles skips (so the working tree and the --base read
agree), and Up(text), which blanks a down section line for line before
splitting, so every remaining statement keeps its line number. The
markers are the tools' documented ones, options after them included
(notransaction, transaction:false).
Not included: Flyway's undo migrations (U<version>__*.sql). They are a
paid Flyway feature and I did not verify their naming from Flyway's
documentation.
Corpus: golang-migrate-down-files (reduced from Mattermost's 000092),
goose-down-section. Both fail on main (a false sink-column-unknown and a
false topic-table-mapping) and pass here. RefuseRadar unchanged at 0/0/159.
avison9
added a commit
that referenced
this pull request
Sep 25, 2026
… source Debezium's MySQL connector is where the most include-list questions come from, and cdclint could only read Postgres. MySQL has no schemas: Debezium names a table database.table and a column database.table.column, so a new reader puts the database where the Postgres reader puts the schema and the rules, include lists and topic names work unchanged. The connector's class picks the reader (MySqlConnector, MariaDbConnector); --migrations mysql:DIR or postgres:DIR says it outright. The database unqualified migrations run in comes from a USE statement, or from a connector whose database.include.list names one database (or whose table.include.list entries all start with the same one); when neither says, cdclint stops and says how to tell it, since guessing would name the wrong table in every finding. database.include.list and database.exclude.list are applied before the table lists, as Debezium does, and sink-table-not-captured names the list that actually keeps a table out, with the entry that belongs in it: "add billing to database.include.list", not billing.invoices. The reader covers CREATE TABLE (backticks, ENGINE options, KEY, INDEX, FULLTEXT and SPATIAL lines, LIKE, keywords glued to their column list), ALTER TABLE (ADD with FIRST or AFTER and several columns in parentheses, CHANGE, MODIFY, DROP, RENAME COLUMN, RENAME TO another database), RENAME TABLE, DROP TABLE and USE. The splitter gains a MySQL mode: # comments, backslash escapes, no dollar quoting, and DELIMITER. MySQL has no ADD COLUMN IF NOT EXISTS, so re-runnable migrations put the DDL in a string run through PREPARE; the reader applies the table statements in those strings, which is the end state the migration guarantees. Proven against a real repository: Mattermost v10.11.0 keeps the same schema as 140 MySQL and 140 Postgres migrations. Both readers find the same 71 tables; of 610 columns, every difference left is on the Postgres reader's side (a glued UNIQUE( read as a column, and columns added inside DO blocks). Before reading the prepared-statement strings the MySQL reader missed about 90 columns there. Down migrations are left out here as in the Postgres reader (#17): golang-migrate *.down.sql files are skipped and goose, sql-migrate and dbmate down sections blanked, through the same internal/migrate calls. Corpus: mysql-column-never-captured, mysql-change-column, mysql-database-not-included, mysql-diff-adds-column. RefuseRadar is unchanged at 0/0/159.
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.
What it changes
cdclint applied every
*.sqlfile in name order, and every statement in each file. A forward migrate never runs a migration's down half, so the schema cdclint read could differ from the one the connector sees. Now the down half is left out:{version}_{title}.down.sqlfiles are skipped.-- +goose Up/-- +goose Down), sql-migrate (-- +migrate Up/-- +migrate Down), dbmate (-- migrate:up/-- migrate:down): the down section of a file is blanked line for line before the file is split, so every remaining statement keeps its line number. Options after a marker (notransaction,transaction:false) are allowed. Markers checked against each tool's README.How bad it was, measured rather than assumed
goose-down-sectiononmain:.down<.up), so it usually undoes something that has not happened yet and does nothing. Not always: Mattermost v10.11.0's000092_add_createat_to_teammembers.down.sqldropsCreateAtfromReactionsinstead ofTeamMembers, and reading it deleted a real column. On Mattermost's 140 MySQL migrations, 1 of 71 tables came out different with the down files read.golang-migrate-down-filesonmain:I had earlier described the golang-migrate case as "any golang-migrate repository reads wrong". That was too strong; the measurement above is the accurate version.
How
A new
internal/migratepackage:Down(path)for golang-migrate files, whichpostgres.ReadFilesskips (one choke point, so the working tree and the--baseread agree), andUp(text), whichpostgres.Applyruns before splitting. The MySQL reader in #16 will call the same two when it is rebased.Verified
internal/migratetests: golang-migrate names (case, backslash paths,countdown.sqlis not a down file), each tool's markers with options, down then up again (dbmate), CRLF line endings, a column nameddown, text with no markers unchanged.golang-migrate-down-files(reduced from Mattermost's 000092) andgoose-down-section. Both fail onmainwith the findings above and pass here; checked by running them againstmain's reader. Every existing entry unchanged.gofmt -l .clean,go vet,go test ./...pass.0 error(s), 0 warning(s), 159 info, unchanged.Not included
Flyway undo migrations (
U<version>__*.sql): a paid Flyway feature, and I did not verify the naming from Flyway's documentation.Merging
Independent of #13, #14, #15. #16 (MySQL) also touches file reading; I will add the two calls to the MySQL reader when rebasing it.