Fixing the varchar size migration bug (Issue #150)#694
Open
Jatanasio wants to merge 1 commit intoash-project:mainfrom
Open
Fixing the varchar size migration bug (Issue #150)#694Jatanasio wants to merge 1 commit intoash-project:mainfrom
Jatanasio wants to merge 1 commit intoash-project:mainfrom
Conversation
Jatanasio
changed the title
zachdaniel
reviewed
Feb 13, 2026
| describe "varchar migration_types on modify" do | ||
| setup do | ||
| on_exit(fn -> | ||
| File.rm_rf!("test_snapshots_path") |
Contributor
There was a problem hiding this comment.
I've updated the testing patterns in this file, please take a look and adopt them to this test 🙇
Contributor
|
I don't think the approach here is quite right. I believe what we need to do is add the size option if one of the following is true:
|
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
2 participants
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.
Contributor checklist
Leave anything that you believe does not apply unchecked.
So the bug as far as I was able to find was due to the migration size emission logic being nested in an if statement that does a snapshot comparison of the difference of the :size in the old and new migrations. The size emission logic relied on there being a difference detected within the two snapshots, so instances with attributes mapped to :text without a size such as with the :string in the bug example provided transitioning to a sized-type such as varchar would not process it properly. So it would simply skip that portion of the alter_opts function altogether.
The new logic checks if the new migration snapshot has a size attribute, then checks if it is different from the old snapshot size. If it does not have a difference in size values, or if the old migration snapshot does not have a size attribute to compare, it then checks if the current migration contains an attribute that should have a size connected to it and if it does then it updates the size column.
I also added various tests for the new logic; such as, updating existing string columns, changing sizes from a varchar that already has a size, if you're trying to update only specific strings among many they don't all get the update, etc.