From bbada710aa648f3a44b633d9d119fe5666b36a1b Mon Sep 17 00:00:00 2001
From: Tomasz Marek Sulima <Eiji7@users.noreply.github.com>
Date: Tue, 29 Oct 2019 06:44:49 +0100
Subject: [PATCH 01/10] [VIP] First version of dynamic command for
 Ecto.Migration Added Ecto.Migration.dynamic/1 macro Added integration test
 for ensuring correct order Added special function clausule in
 Migration.Runner in order to not afftect currect adapters implementations.
 Ensure that {:dynamic, macro, env} command is self-reversible in runner.

TODO:
Discuss the implementation
Documentation and specs

Related issue: #157
---
 integration_test/sql/migration.exs | 44 ++++++++++++++++++++++++++++++
 lib/ecto/migration.ex              |  6 ++++
 lib/ecto/migration/runner.ex       |  6 ++++
 3 files changed, 56 insertions(+)

diff --git a/integration_test/sql/migration.exs b/integration_test/sql/migration.exs
index 2a55a8f7..c522941a 100644
--- a/integration_test/sql/migration.exs
+++ b/integration_test/sql/migration.exs
@@ -330,8 +330,33 @@ defmodule Ecto.Integration.MigrationTest do
     end
   end
 
+  defmodule MigrationWithDynamicCommand do
+    use Ecto.Migration
+
+    @disable_ddl_transaction true
+
+    @migrate_first "select 'This is a first part of ecto.migrate';"
+    @migrate_second "select 'This is a second part of ecto.migrate';"
+    @rollback_first "select 'This is a first part of ecto.rollback';"
+    @rollback_second "select 'This is a second part of ecto.rollback';"
+
+    def change do
+      execute @migrate_first, @rollback_second
+
+      dynamic do
+        case direction() do
+          :up -> Ecto.Adapters.SQL.query!(repo(), "select 'In the middle of ecto.migrate';", [], [log: :info])
+          :down -> Ecto.Adapters.SQL.query!(repo(), "select 'In the middle of ecto.rollback';", [], [log: :info])
+        end
+      end
+
+      execute @migrate_second, @rollback_first
+    end
+  end
+
   import Ecto.Query, only: [from: 2]
   import Ecto.Migrator, only: [up: 4, down: 4]
+  import ExUnit.CaptureLog, only: [capture_log: 1]
 
   # Avoid migration out of order warnings
   @moduletag :capture_log
@@ -341,6 +366,25 @@ defmodule Ecto.Integration.MigrationTest do
     {:ok, migration_number: System.unique_integer([:positive]) + @base_migration}
   end
 
+  @tag :current
+  test "migration with dynamic", %{migration_number: num} do
+    level = :info
+    args = [PoolRepo, num, MigrationWithDynamicCommand, [log: level]]
+
+    for {name, direction} <- [migrate: :up, rollback: :down] do
+      output = capture_log(fn ->
+        :ok = apply(Ecto.Migrator, direction, args)
+      end)
+
+      lines = String.split(output, "\n")
+      assert Enum.at(lines, 4) =~ "== Running #{num} #{inspect(MigrationWithDynamicCommand)}.change/0"
+      assert Enum.at(lines, 6) =~ ~s[execute "select 'This is a first part of ecto.#{name}';"]
+      assert Enum.at(lines, 9) =~ "select 'In the middle of ecto.#{name}'; []"
+      assert Enum.at(lines, 11) =~ ~s[execute "select 'This is a second part of ecto.#{name}';"]
+      assert Enum.at(lines, 13) =~ ~r"Migrated #{num} in \d.\ds"
+    end
+  end
+
   test "create and drop table and indexes", %{migration_number: num} do
     assert :ok == up(PoolRepo, num, CreateMigration, log: false)
     assert :ok == down(PoolRepo, num, CreateMigration, log: false)
diff --git a/lib/ecto/migration.ex b/lib/ecto/migration.ex
index 2d1d8228..97c435b7 100644
--- a/lib/ecto/migration.ex
+++ b/lib/ecto/migration.ex
@@ -466,6 +466,12 @@ defmodule Ecto.Migration do
     end
   end
 
+  defmacro dynamic(do: block) do
+    quote bind_quoted: [block: Macro.escape(block)] do
+      Runner.execute({:dynamic, block, __ENV__})
+    end
+  end
+
   @doc """
   Creates one of the following:
 
diff --git a/lib/ecto/migration/runner.ex b/lib/ecto/migration/runner.ex
index 813c7006..f04320da 100644
--- a/lib/ecto/migration/runner.ex
+++ b/lib/ecto/migration/runner.ex
@@ -216,6 +216,7 @@ defmodule Ecto.Migration.Runner do
     end
   end
 
+  defp reverse({:dynamic, _macro, _env} = dynamic), do: dynamic
   defp reverse({:create, %Index{} = index}),
     do: {:drop, index}
   defp reverse({:create_if_not_exists, %Index{} = index}),
@@ -332,6 +333,11 @@ defmodule Ecto.Migration.Runner do
     log_and_execute_ddl(repo, log, command)
   end
 
+  defp log_and_execute_ddl(_repo, _log, {:dynamic, macro, env}) do
+    Code.eval_quoted(macro, [], env)
+    :ok
+  end
+
   defp log_and_execute_ddl(repo, %{level: level, sql: sql}, command) do
     log(level, command(command))
     meta = Ecto.Adapter.lookup_meta(repo.get_dynamic_repo())

From ac4560bf81598a2686fd033a8576797937cef6b2 Mon Sep 17 00:00:00 2001
From: Tomasz Marek Sulima <Eiji7@users.noreply.github.com>
Date: Tue, 29 Oct 2019 10:05:24 +0100
Subject: [PATCH 02/10] Add bindings support

---
 integration_test/sql/migration.exs | 8 +++++---
 lib/ecto/migration.ex              | 6 +++---
 lib/ecto/migration/runner.ex       | 6 +++---
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/integration_test/sql/migration.exs b/integration_test/sql/migration.exs
index c522941a..c5e71578 100644
--- a/integration_test/sql/migration.exs
+++ b/integration_test/sql/migration.exs
@@ -336,17 +336,19 @@ defmodule Ecto.Integration.MigrationTest do
     @disable_ddl_transaction true
 
     @migrate_first "select 'This is a first part of ecto.migrate';"
+    @migrate_middle "select 'In the middle of ecto.migrate';"
     @migrate_second "select 'This is a second part of ecto.migrate';"
     @rollback_first "select 'This is a first part of ecto.rollback';"
+    @rollback_middle "select 'In the middle of ecto.rollback';"
     @rollback_second "select 'This is a second part of ecto.rollback';"
 
     def change do
       execute @migrate_first, @rollback_second
 
-      dynamic do
+      dynamic migrate_middle: @migrate_middle, rollback_middle: @rollback_middle do
         case direction() do
-          :up -> Ecto.Adapters.SQL.query!(repo(), "select 'In the middle of ecto.migrate';", [], [log: :info])
-          :down -> Ecto.Adapters.SQL.query!(repo(), "select 'In the middle of ecto.rollback';", [], [log: :info])
+          :up -> Ecto.Adapters.SQL.query!(repo(), migrate_middle, [], [log: :info])
+          :down -> Ecto.Adapters.SQL.query!(repo(), rollback_middle, [], [log: :info])
         end
       end
 
diff --git a/lib/ecto/migration.ex b/lib/ecto/migration.ex
index 97c435b7..19dc0373 100644
--- a/lib/ecto/migration.ex
+++ b/lib/ecto/migration.ex
@@ -466,9 +466,9 @@ defmodule Ecto.Migration do
     end
   end
 
-  defmacro dynamic(do: block) do
-    quote bind_quoted: [block: Macro.escape(block)] do
-      Runner.execute({:dynamic, block, __ENV__})
+  defmacro dynamic(bindings, do: block) do
+    quote bind_quoted: [bindings: bindings, block: Macro.escape(block)] do
+      Runner.execute({:dynamic, block, bindings, __ENV__})
     end
   end
 
diff --git a/lib/ecto/migration/runner.ex b/lib/ecto/migration/runner.ex
index f04320da..d95f2772 100644
--- a/lib/ecto/migration/runner.ex
+++ b/lib/ecto/migration/runner.ex
@@ -216,7 +216,7 @@ defmodule Ecto.Migration.Runner do
     end
   end
 
-  defp reverse({:dynamic, _macro, _env} = dynamic), do: dynamic
+  defp reverse({:dynamic, _macro, _bindings, _env} = dynamic), do: dynamic
   defp reverse({:create, %Index{} = index}),
     do: {:drop, index}
   defp reverse({:create_if_not_exists, %Index{} = index}),
@@ -333,8 +333,8 @@ defmodule Ecto.Migration.Runner do
     log_and_execute_ddl(repo, log, command)
   end
 
-  defp log_and_execute_ddl(_repo, _log, {:dynamic, macro, env}) do
-    Code.eval_quoted(macro, [], env)
+  defp log_and_execute_ddl(_repo, _log, {:dynamic, macro, bindings, env}) do
+    Code.eval_quoted(macro, bindings, env)
     :ok
   end
 

From 8f9b5071d11a72eb67db28acef82fead7714182a Mon Sep 17 00:00:00 2001
From: Tomasz Marek Sulima <Eiji7@users.noreply.github.com>
Date: Tue, 29 Oct 2019 10:11:07 +0100
Subject: [PATCH 03/10] Make bindings optional by adding empty list as argument
 default value.

---
 lib/ecto/migration.ex | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ecto/migration.ex b/lib/ecto/migration.ex
index 19dc0373..50eadfb3 100644
--- a/lib/ecto/migration.ex
+++ b/lib/ecto/migration.ex
@@ -466,7 +466,7 @@ defmodule Ecto.Migration do
     end
   end
 
-  defmacro dynamic(bindings, do: block) do
+  defmacro dynamic(bindings \\ [], do: block) do
     quote bind_quoted: [bindings: bindings, block: Macro.escape(block)] do
       Runner.execute({:dynamic, block, bindings, __ENV__})
     end

From 6491eff502e995fd91cc543d0757032c66567601 Mon Sep 17 00:00:00 2001
From: Tomasz Marek Sulima <Eiji7@users.noreply.github.com>
Date: Thu, 31 Oct 2019 11:02:52 +0100
Subject: [PATCH 04/10] Removed no longer needed @tag and added documentation.

---
 integration_test/sql/migration.exs |  6 +-----
 lib/ecto/migration.ex              | 29 +++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/integration_test/sql/migration.exs b/integration_test/sql/migration.exs
index c5e71578..e84d6c93 100644
--- a/integration_test/sql/migration.exs
+++ b/integration_test/sql/migration.exs
@@ -368,16 +368,12 @@ defmodule Ecto.Integration.MigrationTest do
     {:ok, migration_number: System.unique_integer([:positive]) + @base_migration}
   end
 
-  @tag :current
   test "migration with dynamic", %{migration_number: num} do
     level = :info
     args = [PoolRepo, num, MigrationWithDynamicCommand, [log: level]]
 
     for {name, direction} <- [migrate: :up, rollback: :down] do
-      output = capture_log(fn ->
-        :ok = apply(Ecto.Migrator, direction, args)
-      end)
-
+      output = capture_log(fn -> :ok = apply(Ecto.Migrator, direction, args) end)
       lines = String.split(output, "\n")
       assert Enum.at(lines, 4) =~ "== Running #{num} #{inspect(MigrationWithDynamicCommand)}.change/0"
       assert Enum.at(lines, 6) =~ ~s[execute "select 'This is a first part of ecto.#{name}';"]
diff --git a/lib/ecto/migration.ex b/lib/ecto/migration.ex
index 50eadfb3..106cbed9 100644
--- a/lib/ecto/migration.ex
+++ b/lib/ecto/migration.ex
@@ -466,6 +466,35 @@ defmodule Ecto.Migration do
     end
   end
 
+  @doc """
+  Creates a dynamic command.
+
+  Unlike `flush/0` it's reversible, but migrations inside it like `execute/2`
+  are not supported. This is useful for migration `repo/0` features and
+  runtime generated database queries by `Ecto.Adapters.SQL.query/4` function.
+
+  ## Examples
+
+      create table("posts") do
+        add :body, :string
+      end
+
+      dynamic key: value do
+        Ecto.Adapters.SQL.query!(repo(), "select 'Using binding key: \#{key}';", [], [log: :info])
+        Repo.insert!(%Post{body: key})
+      end
+
+  If you are working with reversible migrations `direction/0` may be useful for you.
+
+      dynamic key: value do
+        if direction() == :up do
+          Ecto.Adapters.SQL.query!(repo(), "select 'Forward: \#{key}';", [], [log: :info])
+        else
+          Ecto.Adapters.SQL.query!(repo(), "select 'Backward: \#{key}';", [], [log: :info])
+        end
+      end
+  """
+  @spec dynamic(bindings :: keyword(), [do: Macro.expr()]) :: Macro.expr()
   defmacro dynamic(bindings \\ [], do: block) do
     quote bind_quoted: [bindings: bindings, block: Macro.escape(block)] do
       Runner.execute({:dynamic, block, bindings, __ENV__})

From c838a8000f8b36c63e6969eda09680758194785a Mon Sep 17 00:00:00 2001
From: Tomasz Marek Sulima <Eiji7@users.noreply.github.com>
Date: Thu, 31 Oct 2019 16:35:24 +0100
Subject: [PATCH 05/10] Changed dynamic/2 macro to dynamic/1 function which
 accepts anonymous function with arity 0 or 1. Function with arity 1 would
 accept optional migration direction which would make migrations shorter.
 Updated documentation and tests.

---
 integration_test/sql/migration.exs | 31 +++++++++++++++++++-----------
 lib/ecto/migration.ex              | 31 +++++++++++++++---------------
 lib/ecto/migration/runner.ex       | 11 ++++++++---
 3 files changed, 43 insertions(+), 30 deletions(-)

diff --git a/integration_test/sql/migration.exs b/integration_test/sql/migration.exs
index e84d6c93..c501e289 100644
--- a/integration_test/sql/migration.exs
+++ b/integration_test/sql/migration.exs
@@ -333,6 +333,10 @@ defmodule Ecto.Integration.MigrationTest do
   defmodule MigrationWithDynamicCommand do
     use Ecto.Migration
 
+    alias Ecto.Adapters.SQL
+
+    require Logger
+
     @disable_ddl_transaction true
 
     @migrate_first "select 'This is a first part of ecto.migrate';"
@@ -344,16 +348,13 @@ defmodule Ecto.Integration.MigrationTest do
 
     def change do
       execute @migrate_first, @rollback_second
-
-      dynamic migrate_middle: @migrate_middle, rollback_middle: @rollback_middle do
-        case direction() do
-          :up -> Ecto.Adapters.SQL.query!(repo(), migrate_middle, [], [log: :info])
-          :down -> Ecto.Adapters.SQL.query!(repo(), rollback_middle, [], [log: :info])
-        end
-      end
-
+      dynamic(fn -> Logger.info("This is a middle part called by dynamic/0") end)
+      dynamic(&dynamic_query/1)
       execute @migrate_second, @rollback_first
     end
+
+    defp dynamic_query(:up), do: SQL.query!(repo(), @migrate_middle, [], [log: :info])
+    defp dynamic_query(:down), do: SQL.query!(repo(), @rollback_middle, [], [log: :info])
   end
 
   import Ecto.Query, only: [from: 2]
@@ -368,6 +369,7 @@ defmodule Ecto.Integration.MigrationTest do
     {:ok, migration_number: System.unique_integer([:positive]) + @base_migration}
   end
 
+  @tag :current
   test "migration with dynamic", %{migration_number: num} do
     level = :info
     args = [PoolRepo, num, MigrationWithDynamicCommand, [log: level]]
@@ -377,12 +379,19 @@ defmodule Ecto.Integration.MigrationTest do
       lines = String.split(output, "\n")
       assert Enum.at(lines, 4) =~ "== Running #{num} #{inspect(MigrationWithDynamicCommand)}.change/0"
       assert Enum.at(lines, 6) =~ ~s[execute "select 'This is a first part of ecto.#{name}';"]
-      assert Enum.at(lines, 9) =~ "select 'In the middle of ecto.#{name}'; []"
-      assert Enum.at(lines, 11) =~ ~s[execute "select 'This is a second part of ecto.#{name}';"]
-      assert Enum.at(lines, 13) =~ ~r"Migrated #{num} in \d.\ds"
+      {first_line, second_line} = if direction == :up, do: {8, 11}, else: {9, 11}
+      assert Enum.at(lines, first_line) =~ get_middle_log(direction, first_line, name)
+      assert Enum.at(lines, second_line) =~ get_middle_log(direction, second_line, name)
+      assert Enum.at(lines, 13) =~ ~s[execute "select 'This is a second part of ecto.#{name}';"]
+      assert Enum.at(lines, 15) =~ ~r"Migrated #{num} in \d.\ds"
     end
   end
 
+  defp get_middle_log(:up, 8, _name), do: "This is a middle part called by dynamic/0"
+  defp get_middle_log(:up, 11, name), do: "select 'In the middle of ecto.#{name}'; []"
+  defp get_middle_log(:down, 9, name), do: get_middle_log(:up, 11, name)
+  defp get_middle_log(:down, 11, name), do: get_middle_log(:up, 8, name)
+
   test "create and drop table and indexes", %{migration_number: num} do
     assert :ok == up(PoolRepo, num, CreateMigration, log: false)
     assert :ok == down(PoolRepo, num, CreateMigration, log: false)
diff --git a/lib/ecto/migration.ex b/lib/ecto/migration.ex
index 106cbed9..b74acd83 100644
--- a/lib/ecto/migration.ex
+++ b/lib/ecto/migration.ex
@@ -479,27 +479,26 @@ defmodule Ecto.Migration do
         add :body, :string
       end
 
-      dynamic key: value do
+      dynamic(fn ->
         Ecto.Adapters.SQL.query!(repo(), "select 'Using binding key: \#{key}';", [], [log: :info])
         Repo.insert!(%Post{body: key})
-      end
+      end)
 
-  If you are working with reversible migrations `direction/0` may be useful for you.
+  You can optionally accept an extra `direction` argument:
 
-      dynamic key: value do
-        if direction() == :up do
-          Ecto.Adapters.SQL.query!(repo(), "select 'Forward: \#{key}';", [], [log: :info])
-        else
-          Ecto.Adapters.SQL.query!(repo(), "select 'Backward: \#{key}';", [], [log: :info])
-        end
-      end
+      alias Ecto.Adapters.SQL
+
+      dynamic(&my_dynamic_func/1)
+
+      defp my_dynamic_fync(:up),
+        do: SQL.query!(repo(), "select 'Up: \#{key}';", [], [log: :info])
+
+      defp my_dynamic_fync(:down),
+        do: SQL.query!(repo(), "select 'Down: \#{key}';", [], [log: :info])
   """
-  @spec dynamic(bindings :: keyword(), [do: Macro.expr()]) :: Macro.expr()
-  defmacro dynamic(bindings \\ [], do: block) do
-    quote bind_quoted: [bindings: bindings, block: Macro.escape(block)] do
-      Runner.execute({:dynamic, block, bindings, __ENV__})
-    end
-  end
+  @spec dynamic((-> any) | (:up | :down -> any)) :: :ok
+  def dynamic(func) when is_function(func, 0) or is_function(func, 1),
+    do: Runner.execute({:dynamic, func})
 
   @doc """
   Creates one of the following:
diff --git a/lib/ecto/migration/runner.ex b/lib/ecto/migration/runner.ex
index d95f2772..f2d128d2 100644
--- a/lib/ecto/migration/runner.ex
+++ b/lib/ecto/migration/runner.ex
@@ -216,7 +216,7 @@ defmodule Ecto.Migration.Runner do
     end
   end
 
-  defp reverse({:dynamic, _macro, _bindings, _env} = dynamic), do: dynamic
+  defp reverse({:dynamic, func}), do: {:dynamic, func}
   defp reverse({:create, %Index{} = index}),
     do: {:drop, index}
   defp reverse({:create_if_not_exists, %Index{} = index}),
@@ -333,8 +333,13 @@ defmodule Ecto.Migration.Runner do
     log_and_execute_ddl(repo, log, command)
   end
 
-  defp log_and_execute_ddl(_repo, _log, {:dynamic, macro, bindings, env}) do
-    Code.eval_quoted(macro, bindings, env)
+  defp log_and_execute_ddl(_repo, _log, {:dynamic, func}) when is_function(func, 0) do
+    func.()
+    :ok
+  end
+
+  defp log_and_execute_ddl(_repo, _log, {:dynamic, func}) do
+    func.(migrator_direction())
     :ok
   end
 

From d77536786d24a8b6e4402950e90ae9438e14b656 Mon Sep 17 00:00:00 2001
From: Tomasz Marek Sulima <Eiji7@users.noreply.github.com>
Date: Fri, 1 Nov 2019 10:58:14 +0100
Subject: [PATCH 06/10] Dropped new `dynamic/1` in favor of enhancing already
 existing `execute/1` and `execute/2`. Updated `execute/1` and `execute/2`
 documentation. Updated tests

---
 integration_test/sql/migration.exs | 10 +++---
 lib/ecto/migration.ex              | 52 ++++++++----------------------
 lib/ecto/migration/runner.ex       | 10 ++----
 3 files changed, 22 insertions(+), 50 deletions(-)

diff --git a/integration_test/sql/migration.exs b/integration_test/sql/migration.exs
index c501e289..26acd5b0 100644
--- a/integration_test/sql/migration.exs
+++ b/integration_test/sql/migration.exs
@@ -348,13 +348,13 @@ defmodule Ecto.Integration.MigrationTest do
 
     def change do
       execute @migrate_first, @rollback_second
-      dynamic(fn -> Logger.info("This is a middle part called by dynamic/0") end)
-      dynamic(&dynamic_query/1)
+      execute(fn -> Logger.info("This is a middle part called by execute") end)
+      execute(&execute_up/0, &execute_down/0)
       execute @migrate_second, @rollback_first
     end
 
-    defp dynamic_query(:up), do: SQL.query!(repo(), @migrate_middle, [], [log: :info])
-    defp dynamic_query(:down), do: SQL.query!(repo(), @rollback_middle, [], [log: :info])
+    defp execute_up, do: SQL.query!(repo(), @migrate_middle, [], [log: :info])
+    defp execute_down, do: SQL.query!(repo(), @rollback_middle, [], [log: :info])
   end
 
   import Ecto.Query, only: [from: 2]
@@ -387,7 +387,7 @@ defmodule Ecto.Integration.MigrationTest do
     end
   end
 
-  defp get_middle_log(:up, 8, _name), do: "This is a middle part called by dynamic/0"
+  defp get_middle_log(:up, 8, _name), do: "This is a middle part called by execute"
   defp get_middle_log(:up, 11, name), do: "select 'In the middle of ecto.#{name}'; []"
   defp get_middle_log(:down, 9, name), do: get_middle_log(:up, 11, name)
   defp get_middle_log(:down, 11, name), do: get_middle_log(:up, 8, name)
diff --git a/lib/ecto/migration.ex b/lib/ecto/migration.ex
index b74acd83..cf98d8de 100644
--- a/lib/ecto/migration.ex
+++ b/lib/ecto/migration.ex
@@ -466,40 +466,6 @@ defmodule Ecto.Migration do
     end
   end
 
-  @doc """
-  Creates a dynamic command.
-
-  Unlike `flush/0` it's reversible, but migrations inside it like `execute/2`
-  are not supported. This is useful for migration `repo/0` features and
-  runtime generated database queries by `Ecto.Adapters.SQL.query/4` function.
-
-  ## Examples
-
-      create table("posts") do
-        add :body, :string
-      end
-
-      dynamic(fn ->
-        Ecto.Adapters.SQL.query!(repo(), "select 'Using binding key: \#{key}';", [], [log: :info])
-        Repo.insert!(%Post{body: key})
-      end)
-
-  You can optionally accept an extra `direction` argument:
-
-      alias Ecto.Adapters.SQL
-
-      dynamic(&my_dynamic_func/1)
-
-      defp my_dynamic_fync(:up),
-        do: SQL.query!(repo(), "select 'Up: \#{key}';", [], [log: :info])
-
-      defp my_dynamic_fync(:down),
-        do: SQL.query!(repo(), "select 'Down: \#{key}';", [], [log: :info])
-  """
-  @spec dynamic((-> any) | (:up | :down -> any)) :: :ok
-  def dynamic(func) when is_function(func, 0) or is_function(func, 1),
-    do: Runner.execute({:dynamic, func})
-
   @doc """
   Creates one of the following:
 
@@ -774,7 +740,7 @@ defmodule Ecto.Migration do
   end
 
   @doc """
-  Executes arbitrary SQL or a keyword command.
+  Executes arbitrary SQL, anonymous function or a keyword command.
 
   Reversible commands can be defined by calling `execute/2`.
 
@@ -784,8 +750,12 @@ defmodule Ecto.Migration do
 
       execute create: "posts", capped: true, size: 1024
 
+      execute(fn ->
+        Ecto.Adapters.SQL.query!(repo(), "select 'Using binding key: \#{key}';", [], [log: :info])
+        Repo.insert!(%Post{body: key})
+      end)
   """
-  def execute(command) when is_binary(command) or is_list(command) do
+  def execute(command) when is_binary(command) or is_function(command, 0) or is_list(command) do
     Runner.execute command
   end
 
@@ -802,9 +772,15 @@ defmodule Ecto.Migration do
 
       execute "CREATE EXTENSION postgres_fdw", "DROP EXTENSION postgres_fdw"
 
+      alias Ecto.Adapters.SQL
+
+      execute(&execute_up/0, &execute_down/0)
+
+      defp execute_up, do: SQL.query!(repo(), "select 'Up query …';", [], [log: :info])
+      defp execute_down, do: SQL.query!(repo(), "select 'Down query …';", [], [log: :info])
   """
-  def execute(up, down) when (is_binary(up) or is_list(up)) and
-                             (is_binary(down) or is_list(down)) do
+  def execute(up, down) when (is_binary(up) or is_function(up, 0) or is_list(up)) and
+                             (is_binary(down) or is_function(down, 0) or is_list(down)) do
     Runner.execute %Command{up: up, down: down}
   end
 
diff --git a/lib/ecto/migration/runner.ex b/lib/ecto/migration/runner.ex
index f2d128d2..b305af96 100644
--- a/lib/ecto/migration/runner.ex
+++ b/lib/ecto/migration/runner.ex
@@ -216,7 +216,8 @@ defmodule Ecto.Migration.Runner do
     end
   end
 
-  defp reverse({:dynamic, func}), do: {:dynamic, func}
+  defp reverse(func) when is_function(func, 0), do: func
+
   defp reverse({:create, %Index{} = index}),
     do: {:drop, index}
   defp reverse({:create_if_not_exists, %Index{} = index}),
@@ -333,16 +334,11 @@ defmodule Ecto.Migration.Runner do
     log_and_execute_ddl(repo, log, command)
   end
 
-  defp log_and_execute_ddl(_repo, _log, {:dynamic, func}) when is_function(func, 0) do
+  defp log_and_execute_ddl(_repo, _log, func) when is_function(func, 0) do
     func.()
     :ok
   end
 
-  defp log_and_execute_ddl(_repo, _log, {:dynamic, func}) do
-    func.(migrator_direction())
-    :ok
-  end
-
   defp log_and_execute_ddl(repo, %{level: level, sql: sql}, command) do
     log(level, command(command))
     meta = Ecto.Adapter.lookup_meta(repo.get_dynamic_repo())

From 412cf8fa63126ef9a06022034a1f71532e99072c Mon Sep 17 00:00:00 2001
From: Tomasz Marek Sulima <Eiji7@users.noreply.github.com>
Date: Fri, 1 Nov 2019 13:56:15 +0100
Subject: [PATCH 07/10] Changed migration name to match current implementation
 Moved migration from integration_test/sql/migrations.exs to
 test/ecto/migrator_test.exs Updated "Flushing" -> "Executing and flushing"
 section by adding extra paragraph which mentions difference between those 2
 solutions. Made documentation cleaner by wrapping code inside example module.
 Changed Ecto.Adapters.SQL.query!/4 calls to Logger.info/1 in order to
 increase readability.

---
 integration_test/sql/migration.exs | 51 ------------------------------
 lib/ecto/migration.ex              | 26 ++++++++-------
 test/ecto/migrator_test.exs        | 47 +++++++++++++++++++++++++++
 3 files changed, 62 insertions(+), 62 deletions(-)

diff --git a/integration_test/sql/migration.exs b/integration_test/sql/migration.exs
index 26acd5b0..2a55a8f7 100644
--- a/integration_test/sql/migration.exs
+++ b/integration_test/sql/migration.exs
@@ -330,36 +330,8 @@ defmodule Ecto.Integration.MigrationTest do
     end
   end
 
-  defmodule MigrationWithDynamicCommand do
-    use Ecto.Migration
-
-    alias Ecto.Adapters.SQL
-
-    require Logger
-
-    @disable_ddl_transaction true
-
-    @migrate_first "select 'This is a first part of ecto.migrate';"
-    @migrate_middle "select 'In the middle of ecto.migrate';"
-    @migrate_second "select 'This is a second part of ecto.migrate';"
-    @rollback_first "select 'This is a first part of ecto.rollback';"
-    @rollback_middle "select 'In the middle of ecto.rollback';"
-    @rollback_second "select 'This is a second part of ecto.rollback';"
-
-    def change do
-      execute @migrate_first, @rollback_second
-      execute(fn -> Logger.info("This is a middle part called by execute") end)
-      execute(&execute_up/0, &execute_down/0)
-      execute @migrate_second, @rollback_first
-    end
-
-    defp execute_up, do: SQL.query!(repo(), @migrate_middle, [], [log: :info])
-    defp execute_down, do: SQL.query!(repo(), @rollback_middle, [], [log: :info])
-  end
-
   import Ecto.Query, only: [from: 2]
   import Ecto.Migrator, only: [up: 4, down: 4]
-  import ExUnit.CaptureLog, only: [capture_log: 1]
 
   # Avoid migration out of order warnings
   @moduletag :capture_log
@@ -369,29 +341,6 @@ defmodule Ecto.Integration.MigrationTest do
     {:ok, migration_number: System.unique_integer([:positive]) + @base_migration}
   end
 
-  @tag :current
-  test "migration with dynamic", %{migration_number: num} do
-    level = :info
-    args = [PoolRepo, num, MigrationWithDynamicCommand, [log: level]]
-
-    for {name, direction} <- [migrate: :up, rollback: :down] do
-      output = capture_log(fn -> :ok = apply(Ecto.Migrator, direction, args) end)
-      lines = String.split(output, "\n")
-      assert Enum.at(lines, 4) =~ "== Running #{num} #{inspect(MigrationWithDynamicCommand)}.change/0"
-      assert Enum.at(lines, 6) =~ ~s[execute "select 'This is a first part of ecto.#{name}';"]
-      {first_line, second_line} = if direction == :up, do: {8, 11}, else: {9, 11}
-      assert Enum.at(lines, first_line) =~ get_middle_log(direction, first_line, name)
-      assert Enum.at(lines, second_line) =~ get_middle_log(direction, second_line, name)
-      assert Enum.at(lines, 13) =~ ~s[execute "select 'This is a second part of ecto.#{name}';"]
-      assert Enum.at(lines, 15) =~ ~r"Migrated #{num} in \d.\ds"
-    end
-  end
-
-  defp get_middle_log(:up, 8, _name), do: "This is a middle part called by execute"
-  defp get_middle_log(:up, 11, name), do: "select 'In the middle of ecto.#{name}'; []"
-  defp get_middle_log(:down, 9, name), do: get_middle_log(:up, 11, name)
-  defp get_middle_log(:down, 11, name), do: get_middle_log(:up, 8, name)
-
   test "create and drop table and indexes", %{migration_number: num} do
     assert :ok == up(PoolRepo, num, CreateMigration, log: false)
     assert :ok == down(PoolRepo, num, CreateMigration, log: false)
diff --git a/lib/ecto/migration.ex b/lib/ecto/migration.ex
index cf98d8de..8a21443e 100644
--- a/lib/ecto/migration.ex
+++ b/lib/ecto/migration.ex
@@ -147,7 +147,7 @@ defmodule Ecto.Migration do
   field type with database-specific options, you can pass atoms containing
   these options like `:"int unsigned"`, `:"time without time zone"`, etc.
 
-  ## Flushing
+  ## Executing and flushing
 
   Instructions inside of migrations are not executed immediately. Instead
   they are performed after the relevant `up`, `change`, or `down` callback
@@ -164,6 +164,10 @@ defmodule Ecto.Migration do
         ...
       end
 
+  However `flush/0` will raise if it would be called from `change` function when doing a rollback.
+  To avoid that we recommend to use `execute/2` with anonymous functions instead.
+  For more information and example usage please take a look at `execute/2` function.
+
   ## Comments
 
   Migrations where you create or alter a table support specifying table
@@ -750,10 +754,7 @@ defmodule Ecto.Migration do
 
       execute create: "posts", capped: true, size: 1024
 
-      execute(fn ->
-        Ecto.Adapters.SQL.query!(repo(), "select 'Using binding key: \#{key}';", [], [log: :info])
-        Repo.insert!(%Post{body: key})
-      end)
+      execute(fn -> repo().query!("select 'Anonymous function query …';", [], [log: :info]) end)
   """
   def execute(command) when is_binary(command) or is_function(command, 0) or is_list(command) do
     Runner.execute command
@@ -770,14 +771,17 @@ defmodule Ecto.Migration do
 
   ## Examples
 
-      execute "CREATE EXTENSION postgres_fdw", "DROP EXTENSION postgres_fdw"
-
-      alias Ecto.Adapters.SQL
+      defmodule MyApp.MyMigration do
+        use Ecto.Migration
 
-      execute(&execute_up/0, &execute_down/0)
+        def change do
+          execute "CREATE EXTENSION postgres_fdw", "DROP EXTENSION postgres_fdw"
+          execute(&execute_up/0, &execute_down/0)
+        end
 
-      defp execute_up, do: SQL.query!(repo(), "select 'Up query …';", [], [log: :info])
-      defp execute_down, do: SQL.query!(repo(), "select 'Down query …';", [], [log: :info])
+        defp execute_up, do: repo().query!("select 'Up query …';", [], [log: :info])
+        defp execute_down, do: repo().query!("select 'Down query …';", [], [log: :info])
+      end
   """
   def execute(up, down) when (is_binary(up) or is_function(up, 0) or is_list(up)) and
                              (is_binary(down) or is_function(down, 0) or is_list(down)) do
diff --git a/test/ecto/migrator_test.exs b/test/ecto/migrator_test.exs
index de5cbc8a..4d2c1210 100644
--- a/test/ecto/migrator_test.exs
+++ b/test/ecto/migrator_test.exs
@@ -110,6 +110,31 @@ defmodule Ecto.MigratorTest do
     end
   end
 
+  defmodule AnonymousFunctionMigration do
+    use Ecto.Migration
+
+    require Logger
+
+    @disable_ddl_transaction true
+
+    @migrate_first "select 'This is a first part of ecto.migrate';"
+    @migrate_middle "select 'In the middle of ecto.migrate';"
+    @migrate_second "select 'This is a second part of ecto.migrate';"
+    @rollback_first "select 'This is a first part of ecto.rollback';"
+    @rollback_middle "select 'In the middle of ecto.rollback';"
+    @rollback_second "select 'This is a second part of ecto.rollback';"
+
+    def change do
+      execute @migrate_first, @rollback_second
+      execute(fn -> Logger.info("This is a middle part called by execute") end)
+      execute(&execute_up/0, &execute_down/0)
+      execute @migrate_second, @rollback_first
+    end
+
+    defp execute_up, do: Logger.info(@migrate_middle)
+    defp execute_down, do: Logger.info(@rollback_middle)
+  end
+
   defmodule InvalidMigration do
     use Ecto.Migration
   end
@@ -156,6 +181,28 @@ defmodule Ecto.MigratorTest do
     """
   end
 
+  test "anonymous function migration" do
+    level = :info
+    num = System.unique_integer([:positive])
+    args = [TestRepo, num, AnonymousFunctionMigration, [log: level]]
+
+    for {name, direction} <- [migrate: :up, rollback: :down] do
+      output = capture_log(fn -> :ok = apply(Ecto.Migrator, direction, args) end)
+      lines = String.split(output, "\n")
+      assert Enum.at(lines, 1) =~ "== Running #{num} #{inspect(AnonymousFunctionMigration)}.change/0"
+      assert Enum.at(lines, 3) =~ ~s[execute "select 'This is a first part of ecto.#{name}';"]
+      assert Enum.at(lines, 5) =~ get_middle_log(direction, :first, name)
+      assert Enum.at(lines, 7) =~ get_middle_log(direction, :second, name)
+      assert Enum.at(lines, 9) =~ ~s[execute "select 'This is a second part of ecto.#{name}';"]
+      assert Enum.at(lines, 11) =~ ~r"Migrated #{num} in \d.\ds"
+    end
+  end
+
+  defp get_middle_log(:up, :first, _name), do: "This is a middle part called by execute"
+  defp get_middle_log(:up, :second, name), do: "select 'In the middle of ecto.#{name}';"
+  defp get_middle_log(:down, :first, name), do: get_middle_log(:up, :second, name)
+  defp get_middle_log(:down, :second, name), do: get_middle_log(:up, :first, name)
+
   test "custom schema migrations table is right" do
     assert SchemaMigration.get_source(TestRepo) == "schema_migrations"
     assert SchemaMigration.get_source(MigrationSourceRepo) == "my_schema_migrations"

From df7f6889164e957cc6766bd987b7042288db6ab9 Mon Sep 17 00:00:00 2001
From: Tomasz Marek Sulima <Eiji7@users.noreply.github.com>
Date: Fri, 1 Nov 2019 14:32:05 +0100
Subject: [PATCH 08/10] Remove :current temporary tag

---
 test/ecto/migrator_test.exs | 1 -
 1 file changed, 1 deletion(-)

diff --git a/test/ecto/migrator_test.exs b/test/ecto/migrator_test.exs
index 37942235..8a97039a 100644
--- a/test/ecto/migrator_test.exs
+++ b/test/ecto/migrator_test.exs
@@ -216,7 +216,6 @@ defmodule Ecto.MigratorTest do
   defp get_middle_log(:down, :first, name), do: get_middle_log(:up, :second, name)
   defp get_middle_log(:down, :second, name), do: get_middle_log(:up, :first, name)
 
-  @tag :current
   test "flush" do
     num = System.unique_integer([:positive])
     assert :ok == up(TestRepo, num, EmptyUpDownMigration, log: false)

From 5bfb7d42545aca08748578fa86d15c95224005d9 Mon Sep 17 00:00:00 2001
From: Tomasz Marek Sulima <Eiji7@users.noreply.github.com>
Date: Fri, 1 Nov 2019 16:33:15 +0100
Subject: [PATCH 09/10] Removed incorrect `Ecto.Migration.Runner.reverse/1`
 function clausule. Added test to ensure raising FunctionClauseError. Updated
 test by removing unsupported `execute/1` call on rollback.

---
 lib/ecto/migration/runner.ex |  2 --
 test/ecto/migrator_test.exs  | 49 ++++++++++++++++++++++--------------
 2 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/lib/ecto/migration/runner.ex b/lib/ecto/migration/runner.ex
index b305af96..2e769ea5 100644
--- a/lib/ecto/migration/runner.ex
+++ b/lib/ecto/migration/runner.ex
@@ -216,8 +216,6 @@ defmodule Ecto.Migration.Runner do
     end
   end
 
-  defp reverse(func) when is_function(func, 0), do: func
-
   defp reverse({:create, %Index{} = index}),
     do: {:drop, index}
   defp reverse({:create_if_not_exists, %Index{} = index}),
diff --git a/test/ecto/migrator_test.exs b/test/ecto/migrator_test.exs
index 8a97039a..ea60f827 100644
--- a/test/ecto/migrator_test.exs
+++ b/test/ecto/migrator_test.exs
@@ -110,7 +110,17 @@ defmodule Ecto.MigratorTest do
     end
   end
 
-  defmodule AnonymousFunctionMigration do
+  defmodule ExecuteOneAnonymousFunctionMigration do
+    use Ecto.Migration
+
+    require Logger
+
+    def change do
+      execute(fn -> Logger.info("This should fail on rollback.") end)
+    end
+  end
+
+  defmodule ExecuteTwoAnonymousFunctionsMigration do
     use Ecto.Migration
 
     require Logger
@@ -125,10 +135,9 @@ defmodule Ecto.MigratorTest do
     @rollback_second "select 'This is a second part of ecto.rollback';"
 
     def change do
-      execute @migrate_first, @rollback_second
-      execute(fn -> Logger.info("This is a middle part called by execute") end)
+      execute(@migrate_first, @rollback_second)
       execute(&execute_up/0, &execute_down/0)
-      execute @migrate_second, @rollback_first
+      execute(@migrate_second, @rollback_first)
     end
 
     defp execute_up, do: Logger.info(@migrate_middle)
@@ -161,9 +170,11 @@ defmodule Ecto.MigratorTest do
 
   Application.put_env(:ecto_sql, MigrationSourceRepo, [migration_source: "my_schema_migrations"])
 
+  @base_migration 1_000_000
+
   setup do
     Process.put(:migrated_versions, [1, 2, 3])
-    :ok
+    {:ok, migration_number: System.unique_integer([:positive]) + @base_migration}
   end
 
   def put_test_adapter_config(config) do
@@ -194,28 +205,28 @@ defmodule Ecto.MigratorTest do
     """
   end
 
-  test "anonymous function migration" do
-    level = :info
-    num = System.unique_integer([:positive])
-    args = [TestRepo, num, AnonymousFunctionMigration, [log: level]]
+  test "execute one anonymous function", %{migration_number: num} do
+    module = ExecuteOneAnonymousFunctionMigration
+    capture_log(fn -> :ok = up(TestRepo, num, module, [log: false]) end)
+    message = "no function clause matching in Ecto.Migration.Runner.command/1"
+    assert_raise(FunctionClauseError, message, fn -> down(TestRepo, num, module, [log: false]) end)
+  end
+
+  test "execute two anonymous functions", %{migration_number: num} do
+    module = ExecuteTwoAnonymousFunctionsMigration
+    args = [TestRepo, num, module, [log: :info]]
 
     for {name, direction} <- [migrate: :up, rollback: :down] do
       output = capture_log(fn -> :ok = apply(Ecto.Migrator, direction, args) end)
       lines = String.split(output, "\n")
-      assert Enum.at(lines, 1) =~ "== Running #{num} #{inspect(AnonymousFunctionMigration)}.change/0"
+      assert Enum.at(lines, 1) =~ "== Running #{num} #{inspect(module)}.change/0"
       assert Enum.at(lines, 3) =~ ~s[execute "select 'This is a first part of ecto.#{name}';"]
-      assert Enum.at(lines, 5) =~ get_middle_log(direction, :first, name)
-      assert Enum.at(lines, 7) =~ get_middle_log(direction, :second, name)
-      assert Enum.at(lines, 9) =~ ~s[execute "select 'This is a second part of ecto.#{name}';"]
-      assert Enum.at(lines, 11) =~ ~r"Migrated #{num} in \d.\ds"
+      assert Enum.at(lines, 5) =~ "select 'In the middle of ecto.#{name}';"
+      assert Enum.at(lines, 7) =~ ~s[execute "select 'This is a second part of ecto.#{name}';"]
+      assert Enum.at(lines, 9) =~ ~r"Migrated #{num} in \d.\ds"
     end
   end
 
-  defp get_middle_log(:up, :first, _name), do: "This is a middle part called by execute"
-  defp get_middle_log(:up, :second, name), do: "select 'In the middle of ecto.#{name}';"
-  defp get_middle_log(:down, :first, name), do: get_middle_log(:up, :second, name)
-  defp get_middle_log(:down, :second, name), do: get_middle_log(:up, :first, name)
-
   test "flush" do
     num = System.unique_integer([:positive])
     assert :ok == up(TestRepo, num, EmptyUpDownMigration, log: false)

From e6bb32068d2bf143f2a4fe7024511547c67b7cce Mon Sep 17 00:00:00 2001
From: Tomasz Marek Sulima <Eiji7@users.noreply.github.com>
Date: Fri, 1 Nov 2019 16:42:18 +0100
Subject: [PATCH 10/10] Moved `System.unique_integer/1` from `setup/1` to new
 `test/2`.

---
 test/ecto/migrator_test.exs | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/test/ecto/migrator_test.exs b/test/ecto/migrator_test.exs
index ea60f827..acb43f07 100644
--- a/test/ecto/migrator_test.exs
+++ b/test/ecto/migrator_test.exs
@@ -170,11 +170,9 @@ defmodule Ecto.MigratorTest do
 
   Application.put_env(:ecto_sql, MigrationSourceRepo, [migration_source: "my_schema_migrations"])
 
-  @base_migration 1_000_000
-
   setup do
     Process.put(:migrated_versions, [1, 2, 3])
-    {:ok, migration_number: System.unique_integer([:positive]) + @base_migration}
+    :ok
   end
 
   def put_test_adapter_config(config) do
@@ -205,15 +203,17 @@ defmodule Ecto.MigratorTest do
     """
   end
 
-  test "execute one anonymous function", %{migration_number: num} do
+  test "execute one anonymous function" do
     module = ExecuteOneAnonymousFunctionMigration
+    num = System.unique_integer([:positive])
     capture_log(fn -> :ok = up(TestRepo, num, module, [log: false]) end)
     message = "no function clause matching in Ecto.Migration.Runner.command/1"
     assert_raise(FunctionClauseError, message, fn -> down(TestRepo, num, module, [log: false]) end)
   end
 
-  test "execute two anonymous functions", %{migration_number: num} do
+  test "execute two anonymous functions" do
     module = ExecuteTwoAnonymousFunctionsMigration
+    num = System.unique_integer([:positive])
     args = [TestRepo, num, module, [log: :info]]
 
     for {name, direction} <- [migrate: :up, rollback: :down] do