From 0638ffe32d383bd9d22aaa5cc27ece96c449958c Mon Sep 17 00:00:00 2001 From: Brandur Date: Sat, 30 May 2026 20:06:49 +0200 Subject: [PATCH] Make up migrations with `TargetVersion` idempotent Here, try to address #1260 by making up migrations with `TargetVersion` no longer error if we find that the database has already reached that target version. These now return idempotently instead as a user convenience. Down migrations with `TargetVersion` already worked with this way, so although we add a test case to verify this, we don't have to do anything else. Fixes #1260. --- CHANGELOG.md | 3 ++- rivermigrate/river_migrate.go | 27 ++++++++++++++++----------- rivermigrate/river_migrate_test.go | 16 ++++++++++------ 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01e43314..832ac12d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,9 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ⚠️ **Breaking API change:** `rivermigrate.Migrator.Validate` and `rivermigrate.Migrator.ValidateTx` now take a `*rivermigrate.ValidateOpts` parameter. Pass `nil` to preserve previous behavior. We normally endeavor not to make any breaking API changes, but this one will keep the API in a much nicer state, and is on an ancillary function that most installations won't be using. [PR #1259](https://github.com/riverqueue/river/pull/1259) -### Added +### Changed - Added `rivermigrate.ValidateOpts.TargetVersion` so validation can check migrations up to a specific target version, matching the target-version behavior available on `Migrate` and `MigrateTx`. Notably, this is a breaking API change as the validate functions previously didn't take any options. [PR #1259](https://github.com/riverqueue/river/pull/1259) +- When using `(*Migrator[TTx]).Migrate` with a `TargetVersion` that's already applied, River now no-ops idempotently instead of returning an error as a user convenience. [PR #1260](https://github.com/riverqueue/river/pull/1260) ## [0.38.0] - 2026-05-22 diff --git a/rivermigrate/river_migrate.go b/rivermigrate/river_migrate.go index 9b4db9a8..19d15fdf 100644 --- a/rivermigrate/river_migrate.go +++ b/rivermigrate/river_migrate.go @@ -547,17 +547,22 @@ func (m *Migrator[TTx]) applyMigrations(ctx context.Context, exec riverdriver.Ex targetIndex := slices.IndexFunc(sortedTargetMigrations, func(b Migration) bool { return b.Version == opts.TargetVersion }) if targetIndex == -1 { - return nil, fmt.Errorf("version %d is not in target list of valid migrations to apply", opts.TargetVersion) - } - - // Replace target list with list up to target index. Migrations are - // sorted according to the direction we're migrating in, so when down - // migration, the list is already reversed, so this will truncate it so - // it's the most current migration down to the target. - sortedTargetMigrations = sortedTargetMigrations[0 : targetIndex+1] - - if direction == DirectionDown && len(sortedTargetMigrations) > 0 { - sortedTargetMigrations = sortedTargetMigrations[0 : len(sortedTargetMigrations)-1] + // Error, but only if the migration doesn't exist or was never + // applied on a down migration. Up migrations with TargetVersion + // that's already applied should fall through with a no-op. + if _, ok := m.migrations[opts.TargetVersion]; !ok || direction == DirectionDown { + return nil, fmt.Errorf("version %d is not in target list of valid migrations to apply", opts.TargetVersion) + } + } else { + // Replace target list with list up to target index. Migrations are + // sorted according to the direction we're migrating in, so when down + // migration, the list is already reversed, so this will truncate it so + // it's the most current migration down to the target. + sortedTargetMigrations = sortedTargetMigrations[0 : targetIndex+1] + + if direction == DirectionDown && len(sortedTargetMigrations) > 0 { + sortedTargetMigrations = sortedTargetMigrations[0 : len(sortedTargetMigrations)-1] + } } } diff --git a/rivermigrate/river_migrate_test.go b/rivermigrate/river_migrate_test.go index 3ea3ffee..2539b037 100644 --- a/rivermigrate/river_migrate_test.go +++ b/rivermigrate/river_migrate_test.go @@ -331,6 +331,11 @@ func TestMigrator(t *testing.T) { err = dbExecError(ctx, bundle.driver.GetExecutor(), fmt.Sprintf("SELECT name FROM %s.test_table", bundle.schema)) require.Error(t, err) + + // migrating to the same TargetVersion again should be a no-op + res, err = migrator.Migrate(ctx, DirectionDown, &MigrateOpts{TargetVersion: 4}) + require.NoError(t, err) + require.Empty(t, res.Versions) }) t.Run("MigrateDownWithTargetVersionMinusOne", func(t *testing.T) { @@ -569,6 +574,11 @@ func TestMigrator(t *testing.T) { }) require.NoError(t, err) require.Equal(t, seqOneTo(migrationsBundle.MaxVersion+2), sliceutil.Map(migrations, driverMigrationToInt)) + + // migrating to the same TargetVersion again should be a no-op + res, err = migrator.Migrate(ctx, DirectionUp, &MigrateOpts{TargetVersion: migrationsBundle.MaxVersion + 2}) + require.NoError(t, err) + require.Empty(t, res.Versions) }) t.Run("MigrateUpWithTargetVersionInvalid", func(t *testing.T) { @@ -584,12 +594,6 @@ func TestMigrator(t *testing.T) { _, err := migrator.Migrate(ctx, DirectionUp, &MigrateOpts{TargetVersion: 77}) require.EqualError(t, err, "version 77 is not a valid River migration version") } - - // migration exists but already applied - { - _, err := migrator.Migrate(ctx, DirectionUp, &MigrateOpts{TargetVersion: 3}) - require.EqualError(t, err, "version 3 is not in target list of valid migrations to apply") - } }) t.Run("MigrateUpDryRun", func(t *testing.T) {