Tds query improvements (elixir-ecto#208)

Author: mjaric

integration_test/tds/constraints_test.exs

@@ -0,0 +1,158 @@
+defmodule Ecto.Integration.ConstraintsTest do
+  use ExUnit.Case, async: true
+
+  import Ecto.Migrator, only: [up: 4]
+  alias Ecto.Integration.PoolRepo
+
+  defmodule ConstraintMigration do
+    use Ecto.Migration
+
+    @table table(:constraints_test)
+
+    def change do
+      create @table do
+        add :price, :integer
+        add :from, :integer
+        add :to, :integer
+      end
+      create constraint(@table.name, :cannot_overlap, check: "[from] < [to]")
+      create constraint(@table.name, "positive_price", check: "[price] > 0")
+    end
+  end
+
+  defmodule ConstraintMigration2 do
+    use Ecto.Migration
+
+    def change do
+      opts = [with: "NOCHECK", check: "[from] < 200"]
+      create constraint(:constraints_test, "from_max", opts)
+    end
+  end
+
+  defmodule ConstraintMigration3 do
+    use Ecto.Migration
+
+    def change do
+      drop constraint(:constraints_test, "from_max")
+    end
+  end
+
+  defmodule Constraint do
+    use Ecto.Integration.Schema
+
+    schema "constraints_test" do
+      field :price, :integer
+      field :from, :integer
+      field :to, :integer
+    end
+  end
+
+  @base_migration 2_000_000
+
+  setup_all do
+    ExUnit.CaptureLog.capture_log(fn ->
+      num = @base_migration + System.unique_integer([:positive])
+      up(PoolRepo, num, ConstraintMigration, log: false)
+    end)
+
+    :ok
+  end
+
+  test "creating, using, and dropping an exclusion constraint" do
+    changeset = Ecto.Changeset.change(%Constraint{}, from: 0, to: 10)
+    {:ok, _} = PoolRepo.insert(changeset)
+
+    non_overlapping_changeset = Ecto.Changeset.change(%Constraint{}, from: 11, to: 12)
+    {:ok, _} = PoolRepo.insert(non_overlapping_changeset)
+
+    overlapping_changeset = Ecto.Changeset.change(%Constraint{}, from: 1900, to: 12)
+
+    exception =
+      assert_raise Ecto.ConstraintError, ~r/constraint error when attempting to insert struct/, fn ->
+        PoolRepo.insert(overlapping_changeset)
+      end
+    assert exception.message =~ "cannot_overlap (check_constraint)"
+    assert exception.message =~ "The changeset has not defined any constraint."
+    assert exception.message =~ "call `check_constraint/3`"
+
+    # Seems like below `Ecto.Changeset.check_constraint(:from)` is not valid for some reason,
+    # constrint name is mandatory and ecto raises ArgumentError
+
+    # message = ~r/constraint error when attempting to insert struct/
+    # exception =
+    #   assert_raise Ecto.ConstraintError, message, fn ->
+    #     overlapping_changeset
+    #     |> Ecto.Changeset.check_constraint(:from)
+    #     |> PoolRepo.insert()
+    #   end
+    # assert exception.message =~ "cannot_overlap (check_constraint)"
+
+    {:error, changeset} =
+      overlapping_changeset
+      |> Ecto.Changeset.check_constraint(:from, name: :cannot_overlap)
+      |> PoolRepo.insert()
+    assert changeset.errors == [from: {"is invalid", [constraint: :check, constraint_name: "cannot_overlap"]}]
+    assert changeset.data.__meta__.state == :built
+
+    ExUnit.CaptureLog.capture_log(fn ->
+      # migrate over existing data, it should pass since `with: NOCHECK` is set
+      num = @base_migration + System.unique_integer([:positive])
+      assert :ok == up(PoolRepo, num, ConstraintMigration2, log: false)
+    end)
+
+    # from is greated than max allowed by database, so check constrint should
+    # forbid insert
+    from_max_changeset = Ecto.Changeset.change(%Constraint{}, from: 300, to: 400)
+
+    exception =
+      assert_raise Ecto.ConstraintError, ~r/constraint error when attempting to insert struct/, fn ->
+        PoolRepo.insert(from_max_changeset)
+      end
+    assert exception.message =~ "from_max (check_constraint)"
+    assert exception.message =~ "The changeset has not defined any constraint."
+    assert exception.message =~ "call `check_constraint/3`"
+
+    ExUnit.CaptureLog.capture_log(fn ->
+      num = @base_migration + System.unique_integer([:positive])
+      assert :ok == up(PoolRepo, num, ConstraintMigration3, log: false)
+    end)
+  end
+
+  test "creating, using, and dropping a check constraint" do
+    # When the changeset doesn't expect the db error
+    changeset = Ecto.Changeset.change(%Constraint{}, price: -10)
+    exception =
+      assert_raise Ecto.ConstraintError, ~r/constraint error when attempting to insert struct/, fn ->
+        PoolRepo.insert(changeset)
+      end
+
+    assert exception.message =~ "positive_price (check_constraint)"
+    assert exception.message =~ "The changeset has not defined any constraint."
+    assert exception.message =~ "call `check_constraint/3`"
+
+    # When the changeset does expect the db error, but doesn't give a custom message
+    {:error, changeset} =
+      changeset
+      |> Ecto.Changeset.check_constraint(:price, name: :positive_price)
+      |> PoolRepo.insert()
+    assert changeset.errors == [price: {"is invalid", [constraint: :check, constraint_name: "positive_price"]}]
+    assert changeset.data.__meta__.state == :built
+
+    # When the changeset does expect the db error and gives a custom message
+    changeset = Ecto.Changeset.change(%Constraint{}, price: -10)
+    {:error, changeset} =
+      changeset
+      |> Ecto.Changeset.check_constraint(:price, name: :positive_price, message: "price must be greater than 0")
+      |> PoolRepo.insert()
+    assert changeset.errors == [price: {"price must be greater than 0", [constraint: :check, constraint_name: "positive_price"]}]
+    assert changeset.data.__meta__.state == :built
+
+    # When the change does not violate the check constraint
+    changeset = Ecto.Changeset.change(%Constraint{}, price: 10, from: 100, to: 200)
+    {:ok, changeset} =
+      changeset
+      |> Ecto.Changeset.check_constraint(:price, name: :positive_price, message: "price must be greater than 0")
+      |> PoolRepo.insert()
+    assert is_integer(changeset.id)
+  end
+end

integration_test/tds/migrations_test.exs

@@ -0,0 +1,119 @@
+defmodule Ecto.Integration.MigrationsTest do
+  use ExUnit.Case, async: true
+
+  alias Ecto.Integration.PoolRepo
+
+  # Avoid migration out of order warnings
+  @moduletag :capture_log
+  @base_migration 3_000_000
+
+  setup do
+    {:ok, migration_number: System.unique_integer([:positive]) + @base_migration}
+  end
+
+  defmodule AddColumnIfNotExistsMigration do
+    use Ecto.Migration
+
+    def up do
+      create table(:add_col_if_not_exists_migration)
+
+      alter table(:add_col_if_not_exists_migration) do
+        add_if_not_exists :value, :integer
+        add_if_not_exists :to_be_added, :integer
+      end
+
+      execute "INSERT INTO add_col_if_not_exists_migration (value, to_be_added) VALUES (1, 2)"
+    end
+
+    def down do
+      drop table(:add_col_if_not_exists_migration)
+    end
+  end
+
+  defmodule DropColumnIfExistsMigration do
+    use Ecto.Migration
+
+    def up do
+      create table(:drop_col_if_exists_migration) do
+        add :value, :integer
+        add :to_be_removed, :integer
+      end
+
+      execute "INSERT INTO drop_col_if_exists_migration (value, to_be_removed) VALUES (1, 2)"
+
+      alter table(:drop_col_if_exists_migration) do
+        remove_if_exists :to_be_removed, :integer
+      end
+    end
+
+    def down do
+      drop table(:drop_col_if_exists_migration)
+    end
+  end
+
+  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 NoErrorOnConditionalColumnMigration do
+    use Ecto.Migration
+
+    def up do
+      create table(:no_error_on_conditional_column_migration)
+
+      alter table(:no_error_on_conditional_column_migration) do
+        add_if_not_exists  :value, :integer
+        add_if_not_exists  :value, :integer
+
+        remove_if_exists :value, :integer
+        remove_if_exists :value, :integer
+      end
+    end
+
+    def down do
+      drop table(:no_error_on_conditional_column_migration)
+    end
+  end
+
+  import Ecto.Query, only: [from: 2]
+  import Ecto.Migrator, only: [up: 4, down: 4]
+
+  test "logs MSSQL notice messages" do
+    # comparing to other db engines, in MSSQL there is no option in CREATE statement
+    # if_not_existis, instead we need to use if statement
+    # due this limitation, nothing will be logged in console
+    num = @base_migration + System.unique_integer([:positive])
+    up(PoolRepo, num, DuplicateTableMigration, log: false)
+
+    # check if first attempt in migration created table since seond yields nothing
+    # about skipping in logs
+    q = "SELECT COUNT(*) FROM [INFORMATION_SCHEMA].[TABLES] WHERE [TABLE_NAME] = @1"
+    p = ["duplicate_table"]
+    assert {:ok, %{rows: [[1]]}} = Ecto.Adapters.SQL.query(PoolRepo, q, p)
+  end
+
+  @tag :no_error_on_conditional_column_migration
+  test "add if not exists and drop if exists does not raise on failure", %{migration_number: num} do
+    assert :ok == up(PoolRepo, num, NoErrorOnConditionalColumnMigration, log: false)
+    assert :ok == down(PoolRepo, num, NoErrorOnConditionalColumnMigration, log: false)
+  end
+
+  @tag :add_column_if_not_exists
+  test "add column if not exists", %{migration_number: num} do
+    assert :ok == up(PoolRepo, num, AddColumnIfNotExistsMigration, log: false)
+    assert [2] == PoolRepo.all from p in "add_col_if_not_exists_migration", select: p.to_be_added
+    :ok = down(PoolRepo, num, AddColumnIfNotExistsMigration, log: false)
+  end
+
+  @tag :remove_column_if_exists
+  test "remove column when exists", %{migration_number: num} do
+    assert :ok == up(PoolRepo, num, DropColumnIfExistsMigration, log: false)
+    assert catch_error(PoolRepo.all from p in "drop_col_if_exists_migration", select: p.to_be_removed)
+    :ok = down(PoolRepo, num, DropColumnIfExistsMigration, log: false)
+  end
+end

integration_test/tds/tds_type_test.exs

@@ -0,0 +1,22 @@
+defmodule Ecto.Integration.TdsTypeTest do
+  use ExUnit.Case, async: true
+
+  import Ecto.Type
+  alias Tds.Ecto.VarChar
+  alias Ecto.Adapters.Tds
+
+  @varchar_string "some string"
+
+  test "dumps through the adapter" do
+    assert adapter_dump(Tds, {:map, VarChar}, %{"a" => @varchar_string}) ==
+             {:ok, %{"a" => @varchar_string}}
+  end
+
+  test "loads through the adapter" do
+    assert adapter_load(Tds, {:map, VarChar}, %{"a" => {@varchar_string, :varchar}}) ==
+             {:ok, %{"a" => @varchar_string}}
+
+    assert adapter_load(Tds, {:map, VarChar}, %{"a" => @varchar_string}) ==
+             {:ok, %{"a" => @varchar_string}}
+  end
+end

integration_test/tds/test_helper.exs

@@ -20,8 +20,6 @@ ExUnit.start(
     :uses_msec,
     # Unique index compares even NULL values for post_id, so below fails inserting permalinks without setting valid post_id
     :insert_cell_wise_defaults,
-    # SELECT NOT(t.bool_column) not supported
-    :select_not,
     # MSSQL does not support strings on text fields
     :text_type_as_string,
     # IDENTITY_INSERT ON/OFF  must be manually executed
@@ -38,8 +36,10 @@ ExUnit.start(
     :union_with_literals,
     # inline queries can't use order by
     :inline_order_by,
-    # running destruction of PK columns requires that constraint is dropped first
+    # running destruction of PK columns requires that PK constraint is dropped first
     :alter_primary_key,
+    # running destruction of PK columns requires that PK constraint is dropped first
+    # accessing column. This is same issue as :alter_primary_key
     :modify_column_with_from,
     # below 2 exclusions (in theory) requires filtered unique index on permalinks table post_id column e.g.
     #   CREATE UNIQUE NONCLUSTERED INDEX idx_tbl_TestUnique_ID
@@ -90,7 +90,7 @@ defmodule Ecto.Integration.TestRepo do
     otp_app: :ecto_sql,
     adapter: Ecto.Adapters.Tds
 
-  def uuid, do: Tds.Types.UUID
+  def uuid, do: Tds.Ecto.UUID
 
   def create_prefix(prefix) do
     """
@@ -141,6 +141,15 @@ defmodule Ecto.Integration.Case do
   end
 end
 
+# :dbg.start()
+# :dbg.tracer()
+# :dbg.p(:all,:c)
+# :dbg.tpl(Ecto.Adapters.Tds.Connection, :column_change, :x)
+# :dbg.tpl(Ecto.Adapters.Tds.Connection, :execute_ddl, :x)
+# :dbg.tpl(Ecto.Adapters.Tds.Connection, :all, :x)
+# :dbg.tpl(Tds.Parameter, :prepare_params, :x)
+# :dbg.tpl(Tds.Parameter, :prepared_params, :x)
+
 {:ok, _} = Ecto.Adapters.Tds.ensure_all_started(TestRepo.config(), :temporary)
 
 # Load up the repository, start it, and run migrations