Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: update total statements in the migration when it is being executed #2456

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ruslic19
Copy link

The reason for this is that users are required to edit existing migration files to resolve partial failure issues. Without keeping total in sync, such migration would be considered still pending, because total != applied.

This fix was NOT tested in all possible use cases, but only in the one described in the linked issue and it resolves it.

Issue: #2455

The reason for this is that users are required to edit existing migration files to resolve partial failure issues. Without keeping total in sync, such migration would be considered still pending, because total != applied.
Issue: ariga#2455
@a8m
Copy link
Member

a8m commented Jan 17, 2024

Thanks for opening this PR, @ruslic19. It requires additional work because in cases like the one mentioned in the issue, the "partial hashes" (stored in the revisions table) need to be updated as well.

I suggest using migrate set for now to unblock yourself, and we will provide updates on this matter - we encountered this in the past, and have some ideas how to handled this.

@ruslic19
Copy link
Author

Hi again, @a8m, I just had on my opinion more serious issue than previous one. My partially failed migration was marked as successful even though it didn't successfully end. The reason is still the same.

Steps to reproduce:

  • Declaratively define users table schema with nullable name and age
  • Generate migration from defined schema and execute it
  • Insert couple of rows into users table with name and age being null
  • Change users table schema to have name and age NOT NULL
  • Generate migration from updated schema and try to execute it
  • Migration will be marked as partially failed, because table already contains null values
  • Manually edit migration, but add only one query to update values for name to some default one
  • Recalculate hash of the migrations directory
  • Execute migration again, it fails again, because age still contains null values, but at this point one statement to update name values succeeded
  • Now, revisions's applied statements equal to 1 and total statements are also 1
  • So, migration is now marked as successful, even though it didn't actually complete

So, because of this I decided to take a look on what you said and it seems to me that partial hashes are actually getting updated already, take a look here in the code.

Also checked it manually by inspecting atlas_schema_revisions and every time I execute migration with additional successful query to fix partial failure, new partial hash is getting added to the atlas_schema_revisions.

As I understand, this should be good enough to only add new partial hashes, because previous query should not be edited anyway, because partially failed migration will anyway skip successful queries during next execution, so only queries added after successful one matter.

I can be wrong here of course, so, sorry for bothering in such case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants