avoid alter column type on modify if not needed

elixir-ecto · Oct 17, 2024 · eeed434 · eeed434
1 parent 70a375f
commit eeed434
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 12 deletions.
diff --git a/integration_test/sql/migration.exs b/integration_test/sql/migration.exs
@@ -48,6 +48,7 @@ defmodule Ecto.Integration.MigrationTest do
     def up do
       create table(:alter_col_migration) do
         add :from_null_to_not_null, :integer
+        add :another_from_null_to_not_null, :string
         add :from_not_null_to_null, :integer, null: false
 
         add :from_default_to_no_default, :integer, default: 0
@@ -56,13 +57,14 @@ defmodule Ecto.Integration.MigrationTest do
 
       alter table(:alter_col_migration) do
         modify :from_null_to_not_null, :string, null: false
+        modify :another_from_null_to_not_null, :string, null: false, from: {:string, null: true}
         modify :from_not_null_to_null, :string, null: true
 
         modify :from_default_to_no_default, :integer, default: nil
         modify :from_no_default_to_default, :integer, default: 0
       end
 
-      execute "INSERT INTO alter_col_migration (from_null_to_not_null) VALUES ('foo')"
+      execute "INSERT INTO alter_col_migration (from_null_to_not_null, another_from_null_to_not_null) VALUES ('foo', 'baz')"
     end
 
     def down do

diff --git a/lib/ecto/adapters/postgres/connection.ex b/lib/ecto/adapters/postgres/connection.ex
@@ -1556,15 +1556,34 @@ if Code.ensure_loaded?(Postgrex) do
     end
 
     defp column_change(table, {:modify, name, type, opts}) do
-      [
-        drop_reference_expr(opts[:from], table, name),
-        "ALTER COLUMN ",
-        quote_name(name),
-        " TYPE ",
-        column_type(type, opts),
-        modify_null(name, opts),
-        modify_default(name, type, opts)
-      ]
+      column_type = column_type(type, opts)
+
+      extract_type = fn
+        nil -> nil
+        {type, _} -> type
+        %Reference{} -> nil
+        type -> type
+      end
+
+      from_type = extract_type.(opts[:from])
+
+      if column_type == column_type(from_type, opts) do
+        [
+          drop_reference_expr(opts[:from], table, name),
+          modify_null(name, Keyword.put(opts, :prefix_with_comma, false)),
+          modify_default(name, type, opts)
+        ]
+      else
+        [
+          drop_reference_expr(opts[:from], table, name),
+          "ALTER COLUMN ",
+          quote_name(name),
+          " TYPE ",
+          column_type,
+          modify_null(name, opts),
+          modify_default(name, type, opts)
+        ]
+      end
     end
 
     defp column_change(_table, {:remove, name}), do: ["DROP COLUMN ", quote_name(name)]
@@ -1591,9 +1610,12 @@ if Code.ensure_loaded?(Postgrex) do
       do: ["DROP COLUMN IF EXISTS ", quote_name(name)]
 
     defp modify_null(name, opts) do
+      prefix_with_comma = Keyword.get(opts, :prefix_with_comma, true)
+      prefix = if prefix_with_comma, do: ",", else: ""
+
       case Keyword.get(opts, :null) do
-        true -> [", ALTER COLUMN ", quote_name(name), " DROP NOT NULL"]
-        false -> [", ALTER COLUMN ", quote_name(name), " SET NOT NULL"]
+        true -> [prefix, " ALTER COLUMN ", quote_name(name), " DROP NOT NULL"]
+        false -> [prefix, " ALTER COLUMN ", quote_name(name), " SET NOT NULL"]
         nil -> []
       end
     end