From c480f6a263402390e08394e91b163cd56d23f6eb Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Tue, 20 Jan 2026 15:05:43 -0300 Subject: [PATCH 1/2] Rename migration markers from migrate/rollback to up/down The new `drift:up` and `drift:down` markers align with standard migration terminology used by most database migration tools. The shorter names are also more concise and intuitive. Backward compatibility is preserved by continuing to parse the old `drift:migrate` and `drift:rollback` markers while emitting a deprecation warning to STDERR. Only one warning is shown per file to avoid noise when both deprecated markers are present. Updated all fixture files and documentation to use the new markers. --- README.md | 14 +- spec/drift/migration_spec.cr | 148 ++++++++++++++---- spec/drift/migrator_spec.cr | 38 ++--- .../sequence/20211219152312_create_humans.sql | 4 +- .../sequence/20211220182717_create_pets.sql | 4 +- .../20250302234904_create_employees.sql | 4 +- .../20250302234910_create_audit_log.sql | 4 +- ...0250302234927_create_timestamp_trigger.sql | 4 +- src/drift/commands/new.cr | 4 +- src/drift/migration.cr | 23 ++- src/drift/migrator.cr | 4 +- 11 files changed, 173 insertions(+), 78 deletions(-) diff --git a/README.md b/README.md index 0ddae0f..e9909c5 100644 --- a/README.md +++ b/README.md @@ -29,23 +29,23 @@ against a database. This applies to both [the CLI](#as-tool-cli) and [the library](#as-library-crystal-shard) (Crystal shard). Each migration file must contain two special comments before any SQL -statements: `drift:migrate` to indicate that the following statements should -be executed when migrating and `drift:rollback` to indicate the statements +statements: `drift:up` to indicate that the following statements should +be executed when migrating and `drift:down` to indicate the statements should be executed when rolling back the migration. -Each type (migrate, rollback) within the file can contain multiple SQL +Each type (up, down) within the file can contain multiple SQL statements. Statements can span multiple lines, but all must be properly terminated using `;`. ```sql --- drift:migrate +-- drift:up CREATE TABLE users ( id INTEGER PRIMARY KEY, name TEXT, phone TEXT ); --- drift:rollback +-- drift:down DROP TABLE IF EXISTS users; ``` @@ -54,7 +54,7 @@ For complex SQL statements that include semicolons within the statement itself `-- drift:begin` and `-- drift:end` to wrap the entire statement: ```sql --- drift:migrate +-- drift:up -- drift:begin CREATE TRIGGER update_timestamp AFTER UPDATE ON users BEGIN @@ -72,7 +72,7 @@ VALUES END; -- drift:end --- drift:rollback +-- drift:down DROP TRIGGER IF EXISTS update_timestamp; ``` diff --git a/spec/drift/migration_spec.cr b/spec/drift/migration_spec.cr index 3cc0211..eacc215 100644 --- a/spec/drift/migration_spec.cr +++ b/spec/drift/migration_spec.cr @@ -20,6 +20,23 @@ private def memory_db DB.connect("sqlite3:%3Amemory%3A") end +private def capture_stderr(&) + tempfile = File.tempfile("stderr") + original_stderr = STDERR.dup + STDERR.reopen(tempfile) + begin + yield + ensure + STDERR.flush + STDERR.reopen(original_stderr) + end + tempfile.rewind + content = tempfile.gets_to_end + tempfile.close + tempfile.delete + content +end + describe Drift::Migration do describe ".new" do it "accepts an optional filename" do @@ -32,15 +49,15 @@ describe Drift::Migration do it "accepts multiple statements on each direction" do migration = Drift::Migration.new(1) - migration.add(:migrate, "SELECT 1 AS one;") - migration.add(:migrate, "SELECT 2 AS two;") - migration.add(:rollback, "SELECT 3 AS three;") + migration.add(:up, "SELECT 1 AS one;") + migration.add(:up, "SELECT 2 AS two;") + migration.add(:down, "SELECT 3 AS three;") - migration.statements_for(:migrate).size.should eq(2) - migration.statements_for(:migrate).first.should eq("SELECT 1 AS one;") + migration.statements_for(:up).size.should eq(2) + migration.statements_for(:up).first.should eq("SELECT 1 AS one;") - migration.statements_for(:rollback).size.should eq(1) - migration.statements_for(:rollback).first.should eq("SELECT 3 AS three;") + migration.statements_for(:down).size.should eq(1) + migration.statements_for(:down).first.should eq("SELECT 3 AS three;") end end @@ -58,13 +75,13 @@ describe Drift::Migration do context "(magic comments)" do it "parses statement on a given type" do data = <<-SQL - -- drift:migrate + -- drift:up SELECT 1; SQL migration = Drift::Migration.from_io(data, 1) - migration.statements_for(:migrate).size.should eq(1) - migration.statements_for(:migrate).first.should eq("SELECT 1;") + migration.statements_for(:up).size.should eq(1) + migration.statements_for(:up).first.should eq("SELECT 1;") end it "ignores statements without indicated type" do @@ -73,25 +90,25 @@ describe Drift::Migration do SQL migration = Drift::Migration.from_io(contents, 1) - migration.statements_for(:migrate).should be_empty - migration.statements_for(:rollback).should be_empty + migration.statements_for(:up).should be_empty + migration.statements_for(:down).should be_empty end it "recognizes multiple statements of any type" do data = <<-SQL - -- drift:migrate + -- drift:up SELECT 1; SELECT 2; - -- drift:rollback + -- drift:down SELECT 3; SQL migration = Drift::Migration.from_io(data, 1) - migrate_statements = migration.statements_for(:migrate) + migrate_statements = migration.statements_for(:up) migrate_statements.size.should eq(2) - rollback_statements = migration.statements_for(:rollback) + rollback_statements = migration.statements_for(:down) rollback_statements.size.should eq(1) rollback_statements.first.should eq("SELECT 3;") end @@ -105,25 +122,25 @@ describe Drift::Migration do SQL data = <<-SQL - -- drift:migrate + -- drift:up #{create_statement} CREATE INDEX IF NOT EXISTS idx_humans_name ON humans(name); - -- drift:rollback + -- drift:down DROP TABLE IF EXISTS humans; SQL migration = Drift::Migration.from_io(data, 1) - migration.statements_for(:migrate).size.should eq(2) - migration.statements_for(:rollback).size.should eq(1) + migration.statements_for(:up).size.should eq(2) + migration.statements_for(:down).size.should eq(1) - create_statement = migration.statements_for(:migrate).first + create_statement = migration.statements_for(:up).first create_statement.should eq(create_statement) end it "handles mixing regular statements with multi-statement blocks" do mixed_statement = <<-SQL - -- drift:migrate + -- drift:up CREATE TABLE users ( id INTEGER PRIMARY KEY, name TEXT NOT NULL, @@ -143,7 +160,7 @@ describe Drift::Migration do CREATE INDEX idx_users_name ON users(name); - -- drift:rollback + -- drift:down DROP INDEX IF EXISTS idx_users_name; DROP TRIGGER IF EXISTS set_timestamp_on_insert; DROP TABLE IF EXISTS users; @@ -152,10 +169,73 @@ describe Drift::Migration do migration = Drift::Migration.from_io(mixed_statement, 1) # Should have 3 migrate statements: table creation, trigger, and index - migration.statements_for(:migrate).size.should eq(3) + migration.statements_for(:up).size.should eq(3) # Should have 3 rollback statements: index drop, trigger drop, and table drop - migration.statements_for(:rollback).size.should eq(3) + migration.statements_for(:down).size.should eq(3) + end + end + + context "(deprecated markers)" do + it "outputs deprecation warning for drift:migrate marker" do + data = <<-SQL + -- drift:migrate + SELECT 1; + SQL + + stderr = capture_stderr do + migration = Drift::Migration.from_io(data, 1) + migration.statements_for(:up).size.should eq(1) + end + + stderr.should contain("DEPRECATED: 'drift:migrate' marker in '1' is deprecated, use 'drift:up' instead") + end + + it "outputs deprecation warning for drift:rollback marker" do + data = <<-SQL + -- drift:rollback + SELECT 1; + SQL + + stderr = capture_stderr do + migration = Drift::Migration.from_io(data, 1) + migration.statements_for(:down).size.should eq(1) + end + + stderr.should contain("DEPRECATED: 'drift:rollback' marker in '1' is deprecated, use 'drift:down' instead") + end + + it "outputs deprecation warning only once when both deprecated markers present" do + data = <<-SQL + -- drift:migrate + SELECT 1; + + -- drift:rollback + SELECT 2; + SQL + + stderr = capture_stderr do + migration = Drift::Migration.from_io(data, 1) + migration.statements_for(:up).size.should eq(1) + migration.statements_for(:down).size.should eq(1) + end + + # Should only contain one deprecation warning (the first one encountered) + stderr.scan(/DEPRECATED/).size.should eq(1) + stderr.should contain("drift:migrate") + end + + it "uses filename in deprecation warning when provided" do + data = <<-SQL + -- drift:migrate + SELECT 1; + SQL + + stderr = capture_stderr do + Drift::Migration.from_io(data, 1, "test_migration.sql") + end + + stderr.should contain("'test_migration.sql'") end end end @@ -167,22 +247,22 @@ describe Drift::Migration do migration.id.should eq(20211219152312) migration.filename.should eq("20211219152312_create_humans.sql") - migration.statements_for(:migrate).size.should eq(2) - migration.statements_for(:rollback).size.should eq(2) + migration.statements_for(:up).size.should eq(2) + migration.statements_for(:down).size.should eq(2) end it "loads multi-statement migration with begin/end markers correctly" do file_path = fixture_path("trigger", "20250302234927_create_timestamp_trigger.sql") migration = Drift::Migration.load_file(file_path) - migration.statements_for(:migrate).size.should eq(1) - migrate_stmt = migration.statements_for(:migrate).first + migration.statements_for(:up).size.should eq(1) + migrate_stmt = migration.statements_for(:up).first migrate_stmt.should contain("CREATE TRIGGER update_timestamp") migrate_stmt.should contain("UPDATE employees") migrate_stmt.should contain("INSERT INTO") - migration.statements_for(:rollback).size.should eq(1) - migration.statements_for(:rollback).first.should eq("DROP TRIGGER IF EXISTS update_timestamp;") + migration.statements_for(:down).size.should eq(1) + migration.statements_for(:down).first.should eq("DROP TRIGGER IF EXISTS update_timestamp;") end it "raises error when unable to determine migration ID" do @@ -199,10 +279,10 @@ describe Drift::Migration do db.scalar("SELECT COUNT(id) FROM dummy;").as(Int64).should eq(0) migration = Drift::Migration.new(1) - migration.add(:migrate, "INSERT INTO dummy (value) VALUES (20);") - migration.add(:migrate, "INSERT INTO dummy (value) VALUES (10);") + migration.add(:up, "INSERT INTO dummy (value) VALUES (20);") + migration.add(:up, "INSERT INTO dummy (value) VALUES (10);") - migration.run(:migrate, db) + migration.run(:up, db) values = db.query_all "SELECT value FROM dummy ORDER BY id ASC;", &.read(Int64) values.size.should eq(2) diff --git a/spec/drift/migrator_spec.cr b/spec/drift/migrator_spec.cr index 4cf8f80..04770e6 100644 --- a/spec/drift/migrator_spec.cr +++ b/spec/drift/migrator_spec.cr @@ -226,7 +226,7 @@ describe Drift::Migrator do create_dummy db migration = migrator.context[1] - migration.add(:migrate, "INSERT INTO dummy (value) VALUES (10);") + migration.add(:up, "INSERT INTO dummy (value) VALUES (10);") db.scalar("SELECT COUNT(id) FROM dummy;").as(Int64).should eq(0) migrator.apply(1) @@ -239,8 +239,8 @@ describe Drift::Migrator do create_dummy db migration = migrator.context[1] - migration.add(:migrate, "INSERT INTO dummy (value) VALUES (10);") - migration.add(:migrate, "INSERT INTO foo (value)") + migration.add(:up, "INSERT INTO dummy (value) VALUES (10);") + migration.add(:up, "INSERT INTO foo (value)") db.scalar("SELECT COUNT(id) FROM dummy;").as(Int64).should eq(0) expect_raises(Exception) do @@ -281,7 +281,7 @@ describe Drift::Migrator do create_dummy db m1 = migrator.context[1] - m1.add(:migrate, "INSERT INTO dummy (value) VALUES (10);") + m1.add(:up, "INSERT INTO dummy (value) VALUES (10);") db.scalar("SELECT COUNT(id) FROM drift_migrations;").as(Int64).should eq(1) migrator.apply(1, 3) @@ -304,11 +304,11 @@ describe Drift::Migrator do create_dummy db m1 = migrator.context[1] - m1.add(:migrate, "INSERT INTO dummy (value) VALUES (10);") + m1.add(:up, "INSERT INTO dummy (value) VALUES (10);") m2 = migrator.context[3] - m2.add(:migrate, "INSERT INTO dummy (value) VALUES (20);") - m2.add(:migrate, "INSERT INTO foo (value)") + m2.add(:up, "INSERT INTO dummy (value) VALUES (20);") + m2.add(:up, "INSERT INTO foo (value)") expect_raises(Exception) do migrator.apply(1, 3) @@ -322,7 +322,7 @@ describe Drift::Migrator do create_dummy db migration = migrator.context[1] - migration.add(:migrate, "INSERT INTO dummy (value) VALUES (10);") + migration.add(:up, "INSERT INTO dummy (value) VALUES (10);") migrator.apply(1, 1, 1) db.scalar("SELECT COUNT(id) FROM drift_migrations;").as(Int64).should eq(1) @@ -348,7 +348,7 @@ describe Drift::Migrator do create_dummy db migration = migrator.context[1] - migration.add(:rollback, "INSERT INTO dummy (value) VALUES (10);") + migration.add(:down, "INSERT INTO dummy (value) VALUES (10);") db.scalar("SELECT COUNT(id) FROM dummy;").as(Int64).should eq(0) migrator.rollback(1) @@ -361,7 +361,7 @@ describe Drift::Migrator do create_dummy db migration = migrator.context[2] - migration.add(:rollback, "INSERT INTO dummy (value) VALUES (20);") + migration.add(:down, "INSERT INTO dummy (value) VALUES (20);") db.scalar("SELECT COUNT(id) FROM drift_migrations;").as(Int64).should eq(1) migrator.rollback(2) @@ -375,8 +375,8 @@ describe Drift::Migrator do create_dummy db migration = migrator.context[1] - migration.add(:rollback, "INSERT INTO dummy (value) VALUES (10);") - migration.add(:rollback, "INSERT INTO foo (value)") + migration.add(:down, "INSERT INTO dummy (value) VALUES (10);") + migration.add(:down, "INSERT INTO foo (value)") db.scalar("SELECT COUNT(id) FROM dummy;").as(Int64).should eq(0) expect_raises(Exception) do @@ -395,9 +395,9 @@ describe Drift::Migrator do create_dummy db m1 = migrator.context[1] - m1.add(:rollback, "INSERT INTO dummy (value) VALUES (10);") + m1.add(:down, "INSERT INTO dummy (value) VALUES (10);") m3 = migrator.context[3] - m3.add(:rollback, "INSERT INTO dummy (value) VALUES (30);") + m3.add(:down, "INSERT INTO dummy (value) VALUES (30);") db.scalar("SELECT COUNT(id) FROM dummy;").as(Int64).should eq(0) migrator.rollback(3, 1) @@ -422,7 +422,7 @@ describe Drift::Migrator do create_dummy db migration = migrator.context[1] - migration.add(:rollback, "INSERT INTO dummy (value) VALUES (10);") + migration.add(:down, "INSERT INTO dummy (value) VALUES (10);") db.scalar("SELECT COUNT(id) FROM dummy;").as(Int64).should eq(0) migrator.rollback(1, 1, 1, 1) @@ -436,9 +436,9 @@ describe Drift::Migrator do create_dummy db m1 = migrator.context[1] - m1.add(:rollback, "INSERT INTO dummy (value) VALUES (10);") + m1.add(:down, "INSERT INTO dummy (value) VALUES (10);") m2 = migrator.context[2] - m2.add(:rollback, "INSERT INTO dummy (value) VALUES (20);") + m2.add(:down, "INSERT INTO dummy (value) VALUES (20);") db.scalar("SELECT COUNT(id) FROM dummy;").as(Int64).should eq(0) migrator.rollback(2, 1) @@ -453,9 +453,9 @@ describe Drift::Migrator do create_dummy db m1 = migrator.context[1] - m1.add(:rollback, "INSERT INTO foo (value)") + m1.add(:down, "INSERT INTO foo (value)") m2 = migrator.context[2] - m2.add(:rollback, "INSERT INTO dummy (value) VALUES (10);") + m2.add(:down, "INSERT INTO dummy (value) VALUES (10);") db.scalar("SELECT COUNT(id) FROM dummy;").as(Int64).should eq(0) expect_raises(Exception) do diff --git a/spec/fixtures/sequence/20211219152312_create_humans.sql b/spec/fixtures/sequence/20211219152312_create_humans.sql index d18c9a7..c6f4fb8 100644 --- a/spec/fixtures/sequence/20211219152312_create_humans.sql +++ b/spec/fixtures/sequence/20211219152312_create_humans.sql @@ -1,4 +1,4 @@ --- drift:migrate +-- drift:up CREATE TABLE IF NOT EXISTS humans ( id INTEGER PRIMARY KEY, name TEXT NOT NULL @@ -6,7 +6,7 @@ CREATE TABLE IF NOT EXISTS humans ( CREATE INDEX IF NOT EXISTS idx_humans_name ON humans(name); --- drift:rollback +-- drift:down DROP INDEX IF EXISTS idx_humans_name; DROP TABLE IF EXISTS humans; diff --git a/spec/fixtures/sequence/20211220182717_create_pets.sql b/spec/fixtures/sequence/20211220182717_create_pets.sql index 9aece54..4f34325 100644 --- a/spec/fixtures/sequence/20211220182717_create_pets.sql +++ b/spec/fixtures/sequence/20211220182717_create_pets.sql @@ -1,4 +1,4 @@ --- drift:migrate +-- drift:up CREATE TABLE IF NOT EXISTS pets ( id INTEGER PRIMARY KEY NOT NULL, name TEXT NOT NULL, @@ -12,5 +12,5 @@ CREATE TABLE IF NOT EXISTS pets ( CREATE INDEX IF NOT EXISTS idx_pets_name ON pets(name); CREATE INDEX IF NOT EXISTS idx_pets_owner ON pets(owner_id); --- drift:rollback +-- drift:down DROP TABLE IF EXISTS pets; diff --git a/spec/fixtures/trigger/20250302234904_create_employees.sql b/spec/fixtures/trigger/20250302234904_create_employees.sql index 1b03ba9..d355790 100644 --- a/spec/fixtures/trigger/20250302234904_create_employees.sql +++ b/spec/fixtures/trigger/20250302234904_create_employees.sql @@ -1,4 +1,4 @@ --- drift:migrate +-- drift:up CREATE TABLE IF NOT EXISTS employees ( id INTEGER PRIMARY KEY, name TEXT NOT NULL, @@ -8,5 +8,5 @@ CREATE TABLE IF NOT EXISTS employees ( last_modified TIMESTAMP DEFAULT CURRENT_TIMESTAMP ); --- drift:rollback +-- drift:down DROP TABLE IF EXISTS employees; diff --git a/spec/fixtures/trigger/20250302234910_create_audit_log.sql b/spec/fixtures/trigger/20250302234910_create_audit_log.sql index 09bf7d3..b5ffe5d 100644 --- a/spec/fixtures/trigger/20250302234910_create_audit_log.sql +++ b/spec/fixtures/trigger/20250302234910_create_audit_log.sql @@ -1,4 +1,4 @@ --- drift:migrate +-- drift:up CREATE TABLE IF NOT EXISTS audit_log ( id INTEGER PRIMARY KEY, employee_id INTEGER NOT NULL, @@ -7,5 +7,5 @@ CREATE TABLE IF NOT EXISTS audit_log ( FOREIGN KEY (employee_id) REFERENCES employees (id) ); --- drift:rollback +-- drift:down DROP TABLE IF EXISTS audit_log; diff --git a/spec/fixtures/trigger/20250302234927_create_timestamp_trigger.sql b/spec/fixtures/trigger/20250302234927_create_timestamp_trigger.sql index f8f6538..8330636 100644 --- a/spec/fixtures/trigger/20250302234927_create_timestamp_trigger.sql +++ b/spec/fixtures/trigger/20250302234927_create_timestamp_trigger.sql @@ -1,4 +1,4 @@ --- drift:migrate +-- drift:up -- drift:begin CREATE TRIGGER update_timestamp AFTER UPDATE ON employees BEGIN @@ -16,5 +16,5 @@ VALUES END; -- drift:end --- drift:rollback +-- drift:down DROP TRIGGER IF EXISTS update_timestamp; diff --git a/src/drift/commands/new.cr b/src/drift/commands/new.cr index 6203d3e..b353f7d 100644 --- a/src/drift/commands/new.cr +++ b/src/drift/commands/new.cr @@ -18,9 +18,9 @@ module Drift module Commands class New < Command MIGRATION_TEMPLATE = <<-SQL - -- drift:migrate + -- drift:up - -- drift:rollback + -- drift:down SQL def run(*args) diff --git a/src/drift/migration.cr b/src/drift/migration.cr index e5c0fe4..61e23be 100644 --- a/src/drift/migration.cr +++ b/src/drift/migration.cr @@ -16,8 +16,8 @@ module Drift class Migration # :nodoc: enum Type - Migrate - Rollback + Up + Down end # :nodoc: @@ -54,17 +54,32 @@ module Drift buffer = IO::Memory.new type = nil multi_statement_mode = false + deprecated_warning_shown = false io.each_line do |line| stripped_line = line.strip # detect markers if stripped_line.starts_with?(MAGIC_MARKER) case stripped_line[MAGIC_MARKER.size..-1] + when "up" + type = Type::Up + next + when "down" + type = Type::Down + next when "migrate" - type = Type::Migrate + unless deprecated_warning_shown + STDERR.puts "DEPRECATED: 'drift:migrate' marker in '#{filename || id}' is deprecated, use 'drift:up' instead" + deprecated_warning_shown = true + end + type = Type::Up next when "rollback" - type = Type::Rollback + unless deprecated_warning_shown + STDERR.puts "DEPRECATED: 'drift:rollback' marker in '#{filename || id}' is deprecated, use 'drift:down' instead" + deprecated_warning_shown = true + end + type = Type::Down next when "begin" multi_statement_mode = true diff --git a/src/drift/migrator.cr b/src/drift/migrator.cr index 68a661f..2ec446b 100644 --- a/src/drift/migrator.cr +++ b/src/drift/migrator.cr @@ -261,7 +261,7 @@ module Drift # trigger before_apply callbacks @before_apply.each &.call(id) - duration = Time.measure { migration.run(:migrate, cnn) } + duration = Time.measure { migration.run(:up, cnn) } applied_at = Time.utc duration_ns = duration.total_nanoseconds.to_i64 @@ -291,7 +291,7 @@ module Drift @before_rollback.each &.call(id) - duration = Time.measure { migration.run(:rollback, cnn) } + duration = Time.measure { migration.run(:down, cnn) } cnn.exec(sql_delete_migration, id) From 5cfa99a7bd5d3b09e1fdfde2e938783c757fdc79 Mon Sep 17 00:00:00 2001 From: Luis Lavena Date: Tue, 20 Jan 2026 15:08:15 -0300 Subject: [PATCH 2/2] Add missing CHANGELOG entry --- .changes/unreleased/deprecated-20260120-150809.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changes/unreleased/deprecated-20260120-150809.yaml diff --git a/.changes/unreleased/deprecated-20260120-150809.yaml b/.changes/unreleased/deprecated-20260120-150809.yaml new file mode 100644 index 0000000..12aa336 --- /dev/null +++ b/.changes/unreleased/deprecated-20260120-150809.yaml @@ -0,0 +1,5 @@ +kind: deprecated +body: Replace migrate/rollback markers with up/down +time: 2026-01-20T15:08:09.547367-03:00 +custom: + Issue: ""