From 9984b1113c711720ca3842b7c201f5d164485564 Mon Sep 17 00:00:00 2001 From: Jacob Atanasio Date: Fri, 13 Feb 2026 11:21:45 -0700 Subject: [PATCH 1/2] Fixing the varchar size migration bug --- lib/migration_generator/operation.ex | 8 +- test/migration_generator_test.exs | 272 +++++++++++++++++++++++++++ 2 files changed, 276 insertions(+), 4 deletions(-) diff --git a/lib/migration_generator/operation.ex b/lib/migration_generator/operation.ex index 978298d3..8da21c44 100644 --- a/lib/migration_generator/operation.ex +++ b/lib/migration_generator/operation.ex @@ -565,10 +565,10 @@ defmodule AshPostgres.MigrationGenerator.Operation do end size = - if Map.get(attribute, :size) != Map.get(old_attribute, :size) do - if attribute.size do - ", size: #{attribute.size}" - end + if attribute[:size] && + (Map.get(attribute, :size) != Map.get(old_attribute, :size) || + attribute.type in [:varchar, :binary]) do + ", size: #{attribute.size}" end precision = diff --git a/test/migration_generator_test.exs b/test/migration_generator_test.exs index 1f16edcb..2cd0fa38 100644 --- a/test/migration_generator_test.exs +++ b/test/migration_generator_test.exs @@ -3277,6 +3277,278 @@ defmodule AshPostgres.MigrationGeneratorTest do end end + describe "varchar migration_types on modify" do + setup do + on_exit(fn -> + File.rm_rf!("test_snapshots_path") + File.rm_rf!("test_migration_path") + end) + end + + test "modify includes varchar size when adding migration_types to existing string column" do + defresource MyResource do + postgres do + table "my_resources" + repo(AshPostgres.TestRepo) + end + + actions do + defaults([:create, :read, :update, :destroy]) + end + + attributes do + uuid_primary_key(:id) + attribute(:blibs, :string, public?: true) + end + end + + defdomain([MyResource]) + + AshPostgres.MigrationGenerator.generate(Domain, + snapshot_path: "test_snapshots_path", + migration_path: "test_migration_path", + quiet: true, + format: false, + auto_name: true + ) + + defresource MyResource do + postgres do + table "my_resources" + migration_types(blibs: {:varchar, 255}) + repo(AshPostgres.TestRepo) + end + + actions do + defaults([:create, :read, :update, :destroy]) + end + + attributes do + uuid_primary_key(:id) + attribute(:blibs, :string, public?: true) + end + end + + AshPostgres.MigrationGenerator.generate(Domain, + snapshot_path: "test_snapshots_path", + migration_path: "test_migration_path", + quiet: true, + format: false, + auto_name: true + ) + + migration_files = + Enum.sort(Path.wildcard("test_migration_path/**/*_migrate_resources*.exs")) + |> Enum.reject(&String.contains?(&1, "extensions")) + + assert length(migration_files) == 2, + "Expected 2 migrations: initial create + modify. Got #{length(migration_files)}" + + second_migration = File.read!(Enum.at(migration_files, 1)) + + assert second_migration =~ ~S[modify :blibs, :varchar, size: 255], + "Expected modify to include size: 255 for varchar. Migration:\n#{second_migration}" + + assert second_migration =~ ~S[modify :blibs, :text] + end + + test "modify includes new size when changing from one varchar size to another" do + defresource MyResource do + postgres do + table "my_resources_varchar_change" + migration_types(blibs: {:varchar, 100}) + repo(AshPostgres.TestRepo) + end + + actions do + defaults([:create, :read, :update, :destroy]) + end + + attributes do + uuid_primary_key(:id) + attribute(:blibs, :string, public?: true) + end + end + + defdomain([MyResource]) + + AshPostgres.MigrationGenerator.generate(Domain, + snapshot_path: "test_snapshots_path", + migration_path: "test_migration_path", + quiet: true, + format: false, + auto_name: true + ) + + defresource MyResource do + postgres do + table "my_resources_varchar_change" + migration_types(blibs: {:varchar, 255}) + repo(AshPostgres.TestRepo) + end + + actions do + defaults([:create, :read, :update, :destroy]) + end + + attributes do + uuid_primary_key(:id) + attribute(:blibs, :string, public?: true) + end + end + + AshPostgres.MigrationGenerator.generate(Domain, + snapshot_path: "test_snapshots_path", + migration_path: "test_migration_path", + quiet: true, + format: false, + auto_name: true + ) + + migration_files = + Enum.sort(Path.wildcard("test_migration_path/**/*_migrate_resources*.exs")) + |> Enum.reject(&String.contains?(&1, "extensions")) + + assert length(migration_files) == 2 + + second_migration = File.read!(Enum.at(migration_files, 1)) + + assert second_migration =~ ~S[modify :blibs, :varchar, size: 255] + assert second_migration =~ ~S[modify :blibs, :varchar, size: 100] + end + + test "modify includes size when changing text to binary with migration_types" do + defresource MyResource do + postgres do + table "my_resources_binary" + repo(AshPostgres.TestRepo) + end + + actions do + defaults([:create, :read, :update, :destroy]) + end + + attributes do + uuid_primary_key(:id) + attribute(:blobs, :string, public?: true) + end + end + + defdomain([MyResource]) + + AshPostgres.MigrationGenerator.generate(Domain, + snapshot_path: "test_snapshots_path", + migration_path: "test_migration_path", + quiet: true, + format: false, + auto_name: true + ) + + defresource MyResource do + postgres do + table "my_resources_binary" + migration_types(blobs: {:binary, 500}) + repo(AshPostgres.TestRepo) + end + + actions do + defaults([:create, :read, :update, :destroy]) + end + + attributes do + uuid_primary_key(:id) + attribute(:blobs, :string, public?: true) + end + end + + AshPostgres.MigrationGenerator.generate(Domain, + snapshot_path: "test_snapshots_path", + migration_path: "test_migration_path", + quiet: true, + format: false, + auto_name: true + ) + + migration_files = + Enum.sort(Path.wildcard("test_migration_path/**/*_migrate_resources*.exs")) + |> Enum.reject(&String.contains?(&1, "extensions")) + + assert length(migration_files) == 2 + + second_migration = File.read!(Enum.at(migration_files, 1)) + + assert second_migration =~ ~S[modify :blobs, :binary, size: 500] + assert second_migration =~ ~S[modify :blobs, :text] + end + + test "modify only affects attribute with migration_types when multiple string attributes exist" do + defresource MyResource do + postgres do + table "my_resources_multi" + repo(AshPostgres.TestRepo) + end + + actions do + defaults([:create, :read, :update, :destroy]) + end + + attributes do + uuid_primary_key(:id) + attribute(:blibs, :string, public?: true) + attribute(:blobs, :string, public?: true) + end + end + + defdomain([MyResource]) + + AshPostgres.MigrationGenerator.generate(Domain, + snapshot_path: "test_snapshots_path", + migration_path: "test_migration_path", + quiet: true, + format: false, + auto_name: true + ) + + defresource MyResource do + postgres do + table "my_resources_multi" + migration_types(blibs: {:varchar, 255}) + repo(AshPostgres.TestRepo) + end + + actions do + defaults([:create, :read, :update, :destroy]) + end + + attributes do + uuid_primary_key(:id) + attribute(:blibs, :string, public?: true) + attribute(:blobs, :string, public?: true) + end + end + + AshPostgres.MigrationGenerator.generate(Domain, + snapshot_path: "test_snapshots_path", + migration_path: "test_migration_path", + quiet: true, + format: false, + auto_name: true + ) + + migration_files = + Enum.sort(Path.wildcard("test_migration_path/**/*_migrate_resources*.exs")) + |> Enum.reject(&String.contains?(&1, "extensions")) + + assert length(migration_files) == 2 + + second_migration = File.read!(Enum.at(migration_files, 1)) + + assert second_migration =~ ~S[modify :blibs, :varchar, size: 255] + + refute second_migration =~ ~S[modify :blobs] + end + end + describe "create_table_options" do setup do on_exit(fn -> From d33a6c4d02e4830aa93916c6a39066ad8eeb3516 Mon Sep 17 00:00:00 2001 From: Jacob Atanasio Date: Mon, 23 Feb 2026 14:12:41 -0700 Subject: [PATCH 2/2] update the test pattern and function logic --- lib/migration_generator/operation.ex | 2 +- test/migration_generator_test.exs | 103 +++++++++++++-------------- 2 files changed, 51 insertions(+), 54 deletions(-) diff --git a/lib/migration_generator/operation.ex b/lib/migration_generator/operation.ex index 8da21c44..e206ebfe 100644 --- a/lib/migration_generator/operation.ex +++ b/lib/migration_generator/operation.ex @@ -567,7 +567,7 @@ defmodule AshPostgres.MigrationGenerator.Operation do size = if attribute[:size] && (Map.get(attribute, :size) != Map.get(old_attribute, :size) || - attribute.type in [:varchar, :binary]) do + attribute.type != old_attribute.type) do ", size: #{attribute.size}" end diff --git a/test/migration_generator_test.exs b/test/migration_generator_test.exs index 6ce29a78..93fa45a3 100644 --- a/test/migration_generator_test.exs +++ b/test/migration_generator_test.exs @@ -3491,13 +3491,13 @@ defmodule AshPostgres.MigrationGeneratorTest do describe "varchar migration_types on modify" do setup do - on_exit(fn -> - File.rm_rf!("test_snapshots_path") - File.rm_rf!("test_migration_path") - end) + :ok end - test "modify includes varchar size when adding migration_types to existing string column" do + test "modify includes varchar size when adding migration_types to existing string column", %{ + snapshot_path: snapshot_path, + migration_path: migration_path + } do defresource MyResource do postgres do table "my_resources" @@ -3517,8 +3517,8 @@ defmodule AshPostgres.MigrationGeneratorTest do defdomain([MyResource]) AshPostgres.MigrationGenerator.generate(Domain, - snapshot_path: "test_snapshots_path", - migration_path: "test_migration_path", + snapshot_path: snapshot_path, + migration_path: migration_path, quiet: true, format: false, auto_name: true @@ -3542,29 +3542,27 @@ defmodule AshPostgres.MigrationGeneratorTest do end AshPostgres.MigrationGenerator.generate(Domain, - snapshot_path: "test_snapshots_path", - migration_path: "test_migration_path", + snapshot_path: snapshot_path, + migration_path: migration_path, quiet: true, format: false, auto_name: true ) - migration_files = - Enum.sort(Path.wildcard("test_migration_path/**/*_migrate_resources*.exs")) - |> Enum.reject(&String.contains?(&1, "extensions")) - - assert length(migration_files) == 2, - "Expected 2 migrations: initial create + modify. Got #{length(migration_files)}" - - second_migration = File.read!(Enum.at(migration_files, 1)) + assert [_file1, file2] = + Enum.sort(Path.wildcard("#{migration_path}/**/*_migrate_resources*.exs")) + |> Enum.reject(&String.contains?(&1, "extensions")) - assert second_migration =~ ~S[modify :blibs, :varchar, size: 255], - "Expected modify to include size: 255 for varchar. Migration:\n#{second_migration}" + second_migration = File.read!(file2) + assert second_migration =~ ~S[modify :blibs, :varchar, size: 255] assert second_migration =~ ~S[modify :blibs, :text] end - test "modify includes new size when changing from one varchar size to another" do + test "modify includes new size when changing from one varchar size to another", %{ + snapshot_path: snapshot_path, + migration_path: migration_path + } do defresource MyResource do postgres do table "my_resources_varchar_change" @@ -3585,8 +3583,8 @@ defmodule AshPostgres.MigrationGeneratorTest do defdomain([MyResource]) AshPostgres.MigrationGenerator.generate(Domain, - snapshot_path: "test_snapshots_path", - migration_path: "test_migration_path", + snapshot_path: snapshot_path, + migration_path: migration_path, quiet: true, format: false, auto_name: true @@ -3610,26 +3608,27 @@ defmodule AshPostgres.MigrationGeneratorTest do end AshPostgres.MigrationGenerator.generate(Domain, - snapshot_path: "test_snapshots_path", - migration_path: "test_migration_path", + snapshot_path: snapshot_path, + migration_path: migration_path, quiet: true, format: false, auto_name: true ) - migration_files = - Enum.sort(Path.wildcard("test_migration_path/**/*_migrate_resources*.exs")) - |> Enum.reject(&String.contains?(&1, "extensions")) + assert [_file1, file2] = + Enum.sort(Path.wildcard("#{migration_path}/**/*_migrate_resources*.exs")) + |> Enum.reject(&String.contains?(&1, "extensions")) - assert length(migration_files) == 2 - - second_migration = File.read!(Enum.at(migration_files, 1)) + second_migration = File.read!(file2) assert second_migration =~ ~S[modify :blibs, :varchar, size: 255] assert second_migration =~ ~S[modify :blibs, :varchar, size: 100] end - test "modify includes size when changing text to binary with migration_types" do + test "modify includes size when changing text to binary with migration_types", %{ + snapshot_path: snapshot_path, + migration_path: migration_path + } do defresource MyResource do postgres do table "my_resources_binary" @@ -3649,8 +3648,8 @@ defmodule AshPostgres.MigrationGeneratorTest do defdomain([MyResource]) AshPostgres.MigrationGenerator.generate(Domain, - snapshot_path: "test_snapshots_path", - migration_path: "test_migration_path", + snapshot_path: snapshot_path, + migration_path: migration_path, quiet: true, format: false, auto_name: true @@ -3674,26 +3673,27 @@ defmodule AshPostgres.MigrationGeneratorTest do end AshPostgres.MigrationGenerator.generate(Domain, - snapshot_path: "test_snapshots_path", - migration_path: "test_migration_path", + snapshot_path: snapshot_path, + migration_path: migration_path, quiet: true, format: false, auto_name: true ) - migration_files = - Enum.sort(Path.wildcard("test_migration_path/**/*_migrate_resources*.exs")) - |> Enum.reject(&String.contains?(&1, "extensions")) - - assert length(migration_files) == 2 + assert [_file1, file2] = + Enum.sort(Path.wildcard("#{migration_path}/**/*_migrate_resources*.exs")) + |> Enum.reject(&String.contains?(&1, "extensions")) - second_migration = File.read!(Enum.at(migration_files, 1)) + second_migration = File.read!(file2) assert second_migration =~ ~S[modify :blobs, :binary, size: 500] assert second_migration =~ ~S[modify :blobs, :text] end - test "modify only affects attribute with migration_types when multiple string attributes exist" do + test "modify only affects attribute with migration_types when multiple string attributes exist", %{ + snapshot_path: snapshot_path, + migration_path: migration_path + } do defresource MyResource do postgres do table "my_resources_multi" @@ -3714,8 +3714,8 @@ defmodule AshPostgres.MigrationGeneratorTest do defdomain([MyResource]) AshPostgres.MigrationGenerator.generate(Domain, - snapshot_path: "test_snapshots_path", - migration_path: "test_migration_path", + snapshot_path: snapshot_path, + migration_path: migration_path, quiet: true, format: false, auto_name: true @@ -3740,23 +3740,20 @@ defmodule AshPostgres.MigrationGeneratorTest do end AshPostgres.MigrationGenerator.generate(Domain, - snapshot_path: "test_snapshots_path", - migration_path: "test_migration_path", + snapshot_path: snapshot_path, + migration_path: migration_path, quiet: true, format: false, auto_name: true ) - migration_files = - Enum.sort(Path.wildcard("test_migration_path/**/*_migrate_resources*.exs")) - |> Enum.reject(&String.contains?(&1, "extensions")) - - assert length(migration_files) == 2 + assert [_file1, file2] = + Enum.sort(Path.wildcard("#{migration_path}/**/*_migrate_resources*.exs")) + |> Enum.reject(&String.contains?(&1, "extensions")) - second_migration = File.read!(Enum.at(migration_files, 1)) + second_migration = File.read!(file2) assert second_migration =~ ~S[modify :blibs, :varchar, size: 255] - refute second_migration =~ ~S[modify :blobs] end end