Skip to content

Conversation

@realkarych
Copy link

@realkarych realkarych commented Apr 6, 2025

Implements #631 according to #633

@realkarych realkarych changed the title Add MigrateNext option (#631) Add MigrateNext option Apr 6, 2025
@realkarych realkarych marked this pull request as ready for review April 6, 2025 17:04
@realkarych realkarych force-pushed the upgrade-single-migration branch 2 times, most recently from c1cc449 to eaaecec Compare April 6, 2025 19:18
@realkarych realkarych force-pushed the upgrade-single-migration branch from eaaecec to d1bde2f Compare April 6, 2025 19:18
@realkarych
Copy link
Author

@dossy please review

@dossy
Copy link
Collaborator

dossy commented Apr 8, 2025

Code and tests look fair, although questionable whether it's better to parameterize the migrate function vs. duplicating it with minor changes, and tests are passing, which is good.

However, I won't be approving this PR because I recognize that the decision to accept or reject this change needs to be made by someone who is better qualified to decide if this is a good change or not.

See also: #316, #434, #438.

@realkarych
Copy link
Author

Code and tests look fair, although questionable whether it's better to parameterize the migrate function vs. duplicating it with minor changes, and tests are passing, which is good.

However, I won't be approving this PR because I have elected to not involve myself in feature requests of this nature as I have no use for them, and I feel it encourages use of dbmate that are better served by other tools.

See also: #316, #434, #438.

Okey, who can make a decision about this feature? I agree that the option of the migrate command instead migrate-next would be better.

@amacneil Hi! What do you think about that?

@amacneil
Copy link
Owner

There is lots of prior discussion about this:

Including some thoughts I posted in #438 (comment)

I would like to see a proposal that solves all (or most) of these use cases:

  1. Apply or rollback 1 (or n?) pending migrations
  2. Apply or rollback a specific migration
  3. Apply all migrations up to (and including) a specific migration version
  4. Rollback all migrations down to (excluding) a specific migration version
  5. Rollback all migrations

I think we can do this within the scope of the current migrate and rollback commands, without introducing a new command.

For example (open for comments, I think these cover all the needs people have expressed in the past):

# these flags would be supported for `dbmate up` and `dbmate down` too

dbmate migrate               # existing: apply all pending migrations
dbmate rollback              # existing: rollback one version

dbmate migrate --only 0003   # apply only this version
dbmate rollback --only 0003  # rollback only this version

dbmate migrate --to 0003     # migrate up to this version (inclusive)
dbmate rollback --to 0003    # roll back to this version (exclusive, i.e. the specified version will become the current applied version)

dbmate migrate --one         # apply one pending migration
dbmate rollback --one        # this is the default, maybe included for consistency?

dbmate rollback --all        # special case to rollback all migrations
dbmate migrate --all         # this is the default, maybe included for consistency?

@realkarych
Copy link
Author

realkarych commented Apr 29, 2025

There is lots of prior discussion about this:

Including some thoughts I posted in #438 (comment)

I would like to see a proposal that solves all (or most) of these use cases:

  1. Apply or rollback 1 (or n?) pending migrations
  2. Apply or rollback a specific migration
  3. Apply all migrations up to (and including) a specific migration version
  4. Rollback all migrations down to (excluding) a specific migration version
  5. Rollback all migrations

I think we can do this within the scope of the current migrate and rollback commands, without introducing a new command.

For example (open for comments, I think these cover all the needs people have expressed in the past):

# these flags would be supported for `dbmate up` and `dbmate down` too

dbmate migrate               # existing: apply all pending migrations
dbmate rollback              # existing: rollback one version

dbmate migrate --only 0003   # apply only this version
dbmate rollback --only 0003  # rollback only this version

dbmate migrate --to 0003     # migrate up to this version (inclusive)
dbmate rollback --to 0003    # roll back to this version (exclusive, i.e. the specified version will become the current applied version)

dbmate migrate --one         # apply one pending migration
dbmate rollback --one        # this is the default, maybe included for consistency?

dbmate rollback --all        # special case to rollback all migrations
dbmate migrate --all         # this is the default, maybe included for consistency?

So good, I'll implement this in current PR this week.

@realkarych realkarych changed the title Add MigrateNext option Add migrate options: --one, --all, --to Apr 29, 2025
@realkarych realkarych marked this pull request as draft April 29, 2025 19:34
@realkarych realkarych force-pushed the upgrade-single-migration branch 2 times, most recently from 8529880 to 15283a0 Compare April 29, 2025 21:27
@realkarych realkarych force-pushed the upgrade-single-migration branch 2 times, most recently from af5d13c to a47c298 Compare April 29, 2025 21:58
@realkarych realkarych force-pushed the upgrade-single-migration branch from a47c298 to f5fb235 Compare April 29, 2025 22:00
@amacneil
Copy link
Owner

amacneil commented May 1, 2025

epic!

let us know when ready for a review

@realkarych realkarych force-pushed the upgrade-single-migration branch from 061647a to b9aeb2a Compare May 5, 2025 20:04
@realkarych realkarych force-pushed the upgrade-single-migration branch from b847b0e to 3a60709 Compare May 5, 2025 20:17
@realkarych realkarych changed the title Add migrate options: --one, --all, --to Add migrate options: --one, --all, --to, --only May 5, 2025
@realkarych realkarych marked this pull request as ready for review May 5, 2025 20:27
@realkarych realkarych force-pushed the upgrade-single-migration branch from 405dfa6 to ef2ca13 Compare May 5, 2025 20:48
@realkarych
Copy link
Author

@amacneil @dossy Ready for Review! 😊

Copy link

@tjarbo tjarbo left a comment

Choose a reason for hiding this comment

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

Hi, first of all thank you very much for the PR! These features are really missing IMO and are making dbmate a perfect tool for our use-cases.

In order to make my contribution to the project, I had a closer look on the code changes and I have two comments/questions. Hope this can bring the PR a bit closer to get merged in the future!

if m.Applied || m.Version > target {
continue
}
if err := db.MigrateOnly(migrations, m.Version); err != nil {
Copy link

Choose a reason for hiding this comment

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

Compared to the RollbackTo func, the MigrateTo func applies changes even if the intended target does not exists. I would suggest to align the behaviour here and fail with an error msg before any changes are applied if the defined target does not exist.

Copy link
Author

@realkarych realkarych May 22, 2025

Choose a reason for hiding this comment

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

Correct! Thanks for feedback. I fixed this case: added step that checks if target exists in window to apply.

pkg/dbmate/db.go Outdated
)
}

sqlDB, err := db.openDatabaseForMigration(drv)
Copy link

Choose a reason for hiding this comment

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

Any specific reason, why you are reimplementing the "apply migrations steps" (as far as I can see, this is just duplicated code) and not reusing the MigrateOnly function?
Same question of course applies to the Rollback* equivalent functions of course.

Copy link
Author

Choose a reason for hiding this comment

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

No reason :)
Fixed!

@realkarych realkarych force-pushed the upgrade-single-migration branch 2 times, most recently from 89a3ff6 to d698b76 Compare May 22, 2025 21:58
@realkarych
Copy link
Author

@tjarbo Ready for re-review =)

@realkarych realkarych force-pushed the upgrade-single-migration branch from d698b76 to 35ba1c4 Compare May 22, 2025 22:08
@realkarych realkarych force-pushed the upgrade-single-migration branch from 35ba1c4 to 1964c9e Compare May 22, 2025 22:10
@realkarych realkarych requested a review from tjarbo May 22, 2025 22:12
Copy link

@tjarbo tjarbo left a comment

Choose a reason for hiding this comment

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

The code looks good to me. Thanks a lot!
A disclaimer for the the maintainers: I have not tested the code functionally speaking.

I just have another two small comments, but they are not worth to block the PR. I leave it up to you whether to implement them or not.

@tjarbo
Copy link

tjarbo commented Jun 11, 2025

Hi @amacneil
is there anything else I can do to unblock this PR so that it can be merged?

Best regards

@NINNiT
Copy link

NINNiT commented Sep 22, 2025

Thanks for your work - really looking forward to this! It would help immensely in our CI pipelines / tests.

Any news on when this will make it in? 🚀

@realkarych
Copy link
Author

Thanks for your work - really looking forward to this! It would help immensely in our CI pipelines / tests.

Any news on when this will make it in? 🚀

Call for @amacneil =)

I will resolve conflict if this pr will be merged after that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants