From 6b48c5c16a1c8e7c627826f7f3d382c4826b0323 Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Mon, 4 Sep 2023 03:06:42 -0400 Subject: [PATCH 1/3] ddl_transactions --- integration_test/migrations_test.exs | 100 +++++++++++++++++++++++++++ lib/ecto/adapters/sqlite3.ex | 2 +- 2 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 integration_test/migrations_test.exs diff --git a/integration_test/migrations_test.exs b/integration_test/migrations_test.exs new file mode 100644 index 0000000..4853cd5 --- /dev/null +++ b/integration_test/migrations_test.exs @@ -0,0 +1,100 @@ +defmodule Ecto.Integration.MigrationsTest do + use ExUnit.Case, async: true + + alias Ecto.Integration.PoolRepo + import ExUnit.CaptureLog + + @moduletag :capture_log + @base_migration 3_000_000 + + defmodule DuplicateTableMigration do + use Ecto.Migration + + def change do + create_if_not_exists table(:duplicate_table) + create_if_not_exists table(:duplicate_table) + end + end + + defmodule NormalMigration do + use Ecto.Migration + + def change do + create_if_not_exists table(:log_mode_table) + end + end + + defmodule IndexMigration do + use Ecto.Migration + @disable_ddl_transaction true + + def change do + create_if_not_exists table(:index_table) do + add :name, :string + add :custom_id, :uuid + timestamps() + end + + create_if_not_exists index(:index_table, [:name], concurrently: true) + end + end + + describe "Migrator" do + @create_table_sql ~s(CREATE TABLE IF NOT EXISTS "log_mode_table") + @create_table_log "create table if not exists log_mode_table" + @drop_table_sql ~s(DROP TABLE IF EXISTS "log_mode_table") + @drop_table_log "drop table if exists log_mode_table" + @version_insert ~s(INSERT INTO "schema_migrations") + @version_delete ~s(DELETE FROM "schema_migrations") + + test "logs locking and transaction commands" do + num = @base_migration + System.unique_integer([:positive]) + up_log = + capture_log(fn -> + Ecto.Migrator.up(PoolRepo, num, NormalMigration, log_migrator_sql: :info, log_migrations_sql: :info, log: :info) + end) + + assert up_log =~ "begin []" + assert up_log =~ @create_table_sql + assert up_log =~ @create_table_log + assert up_log =~ @version_insert + assert up_log =~ "commit []" + + down_log = + capture_log(fn -> + Ecto.Migrator.down(PoolRepo, num, NormalMigration, log_migrator_sql: :info, log_migrations_sql: :info, log: :info) + end) + + assert down_log =~ "begin []" + assert down_log =~ @drop_table_sql + assert down_log =~ @drop_table_log + assert down_log =~ @version_delete + assert down_log =~ "commit []" + end + + test "does not log sql when log is default" do + num = @base_migration + System.unique_integer([:positive]) + up_log = + capture_log(fn -> + Ecto.Migrator.up(PoolRepo, num, NormalMigration, log: :info) + end) + + refute up_log =~ "begin []" + refute up_log =~ @create_table_sql + assert up_log =~ @create_table_log + refute up_log =~ @version_insert + refute up_log =~ "commit []" + + down_log = + capture_log(fn -> + Ecto.Migrator.down(PoolRepo, num, NormalMigration, log: :info) + end) + + refute down_log =~ "begin []" + refute down_log =~ @drop_table_sql + assert down_log =~ @drop_table_log + refute down_log =~ @version_delete + refute down_log =~ "commit []" + end + end +end \ No newline at end of file diff --git a/lib/ecto/adapters/sqlite3.ex b/lib/ecto/adapters/sqlite3.ex index c6a3f9f..9365d36 100644 --- a/lib/ecto/adapters/sqlite3.ex +++ b/lib/ecto/adapters/sqlite3.ex @@ -233,7 +233,7 @@ defmodule Ecto.Adapters.SQLite3 do end @impl Ecto.Adapter.Migration - def supports_ddl_transaction?, do: false + def supports_ddl_transaction?, do: true @impl Ecto.Adapter.Migration def lock_for_migrations(_meta, _options, fun) do From fe228b8718c6e355aa0c7f5af2d601a1e6986601 Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Mon, 4 Sep 2023 03:23:50 -0400 Subject: [PATCH 2/3] improve test --- integration_test/migrations_test.exs | 35 ++++++++++------------------ 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/integration_test/migrations_test.exs b/integration_test/migrations_test.exs index 4853cd5..626a4bb 100644 --- a/integration_test/migrations_test.exs +++ b/integration_test/migrations_test.exs @@ -7,41 +7,24 @@ defmodule Ecto.Integration.MigrationsTest do @moduletag :capture_log @base_migration 3_000_000 - defmodule DuplicateTableMigration do - use Ecto.Migration - - def change do - create_if_not_exists table(:duplicate_table) - create_if_not_exists table(:duplicate_table) - end - end - defmodule NormalMigration do use Ecto.Migration def change do create_if_not_exists table(:log_mode_table) - end - end - - defmodule IndexMigration do - use Ecto.Migration - @disable_ddl_transaction true - def change do - create_if_not_exists table(:index_table) do - add :name, :string - add :custom_id, :uuid - timestamps() + alter table(:log_mode_table) do + add :name, :text end - - create_if_not_exists index(:index_table, [:name], concurrently: true) end end describe "Migrator" do @create_table_sql ~s(CREATE TABLE IF NOT EXISTS "log_mode_table") @create_table_log "create table if not exists log_mode_table" + @add_column_sql ~S(ALTER TABLE "log_mode_table" ADD COLUMN "name" TEXT) + @alter_table_log "alter table log_mode_table" + @drop_column_sql ~S(ALTER TABLE "log_mode_table" DROP COLUMN "name") @drop_table_sql ~s(DROP TABLE IF EXISTS "log_mode_table") @drop_table_log "drop table if exists log_mode_table" @version_insert ~s(INSERT INTO "schema_migrations") @@ -57,6 +40,8 @@ defmodule Ecto.Integration.MigrationsTest do assert up_log =~ "begin []" assert up_log =~ @create_table_sql assert up_log =~ @create_table_log + assert up_log =~ @add_column_sql + assert up_log =~ @alter_table_log assert up_log =~ @version_insert assert up_log =~ "commit []" @@ -66,6 +51,7 @@ defmodule Ecto.Integration.MigrationsTest do end) assert down_log =~ "begin []" + assert down_log =~ @drop_column_sql assert down_log =~ @drop_table_sql assert down_log =~ @drop_table_log assert down_log =~ @version_delete @@ -82,6 +68,8 @@ defmodule Ecto.Integration.MigrationsTest do refute up_log =~ "begin []" refute up_log =~ @create_table_sql assert up_log =~ @create_table_log + refute up_log =~ @add_column_sql + assert up_log =~ @alter_table_log refute up_log =~ @version_insert refute up_log =~ "commit []" @@ -91,10 +79,11 @@ defmodule Ecto.Integration.MigrationsTest do end) refute down_log =~ "begin []" + refute down_log =~ @drop_column_sql refute down_log =~ @drop_table_sql assert down_log =~ @drop_table_log refute down_log =~ @version_delete refute down_log =~ "commit []" end end -end \ No newline at end of file +end From 400190d8e8c6baedcf74b50522bed170bdaf7052 Mon Sep 17 00:00:00 2001 From: Greg Rychlewski Date: Mon, 4 Sep 2023 04:22:11 -0400 Subject: [PATCH 3/3] improve tests --- integration_test/migrations_test.exs | 39 +++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/integration_test/migrations_test.exs b/integration_test/migrations_test.exs index 626a4bb..90d3c11 100644 --- a/integration_test/migrations_test.exs +++ b/integration_test/migrations_test.exs @@ -19,6 +19,18 @@ defmodule Ecto.Integration.MigrationsTest do end end + defmodule RollbackMigration do + use Ecto.Migration + + def change do + create_if_not_exists table(:log_mode_table) + + alter table(:log_mode_table) do + remove :not_exists, :text + end + end + end + describe "Migrator" do @create_table_sql ~s(CREATE TABLE IF NOT EXISTS "log_mode_table") @create_table_log "create table if not exists log_mode_table" @@ -30,7 +42,7 @@ defmodule Ecto.Integration.MigrationsTest do @version_insert ~s(INSERT INTO "schema_migrations") @version_delete ~s(DELETE FROM "schema_migrations") - test "logs locking and transaction commands" do + test "logs transaction commands" do num = @base_migration + System.unique_integer([:positive]) up_log = capture_log(fn -> @@ -45,6 +57,9 @@ defmodule Ecto.Integration.MigrationsTest do assert up_log =~ @version_insert assert up_log =~ "commit []" + # two columns in the table + assert %{num_rows: 2} = PoolRepo.query!("PRAGMA table_info(log_mode_table)", []) + down_log = capture_log(fn -> Ecto.Migrator.down(PoolRepo, num, NormalMigration, log_migrator_sql: :info, log_migrations_sql: :info, log: :info) @@ -85,5 +100,27 @@ defmodule Ecto.Integration.MigrationsTest do refute down_log =~ @version_delete refute down_log =~ "commit []" end + + test "rolling back undoes previous migrations" do + num = @base_migration + System.unique_integer([:positive]) + up_log = + capture_log(fn -> + try do + Ecto.Migrator.up(PoolRepo, num, RollbackMigration, log_migrator_sql: :info, log_migrations_sql: :info, log: :info) + rescue + _ -> :ok + end + end) + + assert up_log =~ "begin []" + assert up_log =~ @create_table_sql + assert up_log =~ @create_table_log + assert up_log =~ "rollback []" + refute up_log =~ @version_insert + refute up_log =~ "commit []" + + # table was not created due to rollback + assert %{num_rows: 0} = PoolRepo.query!("PRAGMA table_info(log_mode_table)", []) + end end end