Repository: elixir-ecto/ecto
Pull request #2344 (2017-12-12 21:20:02)

Support tuples in filter expressions
MySQL and Postgres support tuple comparisons, as discussed in this mailing list thread: https://groups.google.com/forum/#!topic/elixir-ecto/M5P7RYHg5FU

I've taken a stab at adding support for tuple syntax in queries, so you can do things like `where([a], {a.x, a.y} > {^foo,^bar})`. It should handle casting parameters correctly.

One thing I was not able to do was to make it accept parameters that are tuples. I think it'd be nice if it could do this, so you could do something like `{a.x,a.y} > ^cursor`, but I found the type system in Postgrex particularly hard to make sense of. I could use some help from someone more experienced with it to make that work.

Please review. Thanks!

ebert[bot]: Hello, @JakeBecker! This is your first Pull Request that will be reviewed by [Ebert](https://ebertapp.io/), an automatic Code Review service. It will leave comments on this diff with potential issues and style violations found in the code as you push new commits. You can also see all the issues found on this Pull Request on its [review page](https://ebertapp.io/github/elixir-ecto/ecto/pulls/2344). Please check our [documentation](https://docs.ebertapp.io) for more information. 
josevalim: > One thing I was not able to do was to make it accept parameters that are tuples. I think it'd be nice if it could do this, so you could do something like {a.x,a.y} > ^cursor

Would that be actually valid in Postgres?
ericmj: > Or, to go a bit deeper, how would we encode tuples in Postgrex? 

In postgrex we can decode anonymous composite types to tuples because we have type information for each element in the composite. We cannot encode anonymous composites because we dont have type information for each element.

If the composite type is named, for example a table type, we can encode it but you would have to be explicit with a type cast because postgres can usually not infer composite types.

So in short, no we cant.
JakeBecker: I'm still fuzzy on the type system in Postgrex and on Postgres's binary protocol. Is a tuple significantly different from other parameters? In either case, we can often determine at compile time the type. In `where([x], {x.foo, x.bar} > {^a, ^b}`, it figures out that `a` and `b` correspond to `x.foo` and `x.bar` and should have the same types. That should work if we do `where([x], {x.foo, x.bar} > ^cursor` too, I would have thought.

Is an anonymous tuple possible to encode like this? I took a brief stab at this but I was stymied by the type system with `oid`s and extensions and all.
JakeBecker: Okay, I think I get it. Postgrex gets a `ParameterDescription` message back with an oid of `2249`, which simply indicates a composite type with no indication of what types the elements should have. That makes things more difficult...
josevalim: @JakeBecker How does it work for MySQL? It just works?
JakeBecker: Nope, doesn't seem to work there either. When I try to execute a query with an expression like `where([x], {x.foo, x.bar} > ^cursor)` and `cursor = {"a", "b"}`, I get this error:

```
** (Mariaex.Error) (1241): Operand should contain 2 column(s)
    (ecto) lib/ecto/adapters/sql.ex:428: Ecto.Adapters.SQL.execute_and_cache/6
    (ecto) lib/ecto/adapters/sql.ex:402: Ecto.Adapters.SQL.execute/5
    (ecto) lib/ecto/repo/queryable.ex:133: Ecto.Repo.Queryable.execute/5
    (ecto) lib/ecto/repo/queryable.ex:37: Ecto.Repo.Queryable.all/4
```
josevalim: @JakeBecker so probably we just can't accept it, which I think makes this way less useful?
JakeBecker: I notice that if you do an expression like `from u in User, where: u.email in  ^x` and `x = {"a", "b"}` Mariaex automatically turns that into `SELECT u0.`id`, u0.`email`, u0.`first_name`, u0.`last_name`, u0.`encrypted_password`, u0.`inserted_at`, u0.`updated_at` FROM `users` AS u0 WHERE (u0.`email` IN (?,?)) ["a", "b"]`. It expands the variable into a tuple value and adds `?`s for each element.

Something like that might work for tuples too. I'm not sure where that transformation happens though.
josevalim: @JakeBecker then I would rather force developers to use `{^foo, ^bar}`.
josevalim: Using `{^foo, ^bar}` would also solve the issue with casting. We would never need to cast to a tuple because it exists only in the query syntax.
JakeBecker: Okay, I undid those changes.

Patch:
diff --git a/lib/ecto/adapters/mysql/connection.ex b/lib/ecto/adapters/mysql/connection.ex
index 7bc226aa..0cb7e5a8 100644
--- a/lib/ecto/adapters/mysql/connection.ex
+++ b/lib/ecto/adapters/mysql/connection.ex
@@ -402,6 +402,10 @@ if Code.ensure_loaded?(Mariaex) do
       error!(query, "ilike is not supported by MySQL")
     end
 
+    defp expr({:{}, _, elems}, sources, query) do
+      [?(, intersperse_map(elems, ?,, &expr(&1, sources, query)), ?)]
+    end
+
     defp expr({fun, _, args}, sources, query) when is_atom(fun) and is_list(args) do
       {modifier, args} =
         case args do
diff --git a/lib/ecto/adapters/postgres/connection.ex b/lib/ecto/adapters/postgres/connection.ex
index 8fb2033f..c703f6e7 100644
--- a/lib/ecto/adapters/postgres/connection.ex
+++ b/lib/ecto/adapters/postgres/connection.ex
@@ -470,6 +470,10 @@ if Code.ensure_loaded?(Postgrex) do
        interval(count, interval, sources, query) | ")::date"]
     end
 
+    defp expr({:{}, _, elems}, sources, query) do
+      [?(, intersperse_map(elems, ?,, &expr(&1, sources, query)), ?)]
+    end
+
     defp expr({fun, _, args}, sources, query) when is_atom(fun) and is_list(args) do
       {modifier, args} =
         case args do
diff --git a/lib/ecto/migrator.ex b/lib/ecto/migrator.ex
index a18925f1..18ed1416 100644
--- a/lib/ecto/migrator.ex
+++ b/lib/ecto/migrator.ex
@@ -209,15 +209,10 @@ defmodule Ecto.Migrator do
 
   """
   def migrations(repo, directory) do
-    versions = migrated_versions(repo)
-
-    Enum.map(pending_in_direction(versions, directory, :down) |> Enum.reverse, fn {a, b, _}
-      -> {:up, a, b}
-    end)
-    ++
-    Enum.map(pending_in_direction(versions, directory, :up), fn {a, b, _} ->
-      {:down, a, b}
-    end)
+    repo
+    |> migrated_versions
+    |> collect_migrations(directory)
+    |> Enum.sort_by(fn {_, version, _} -> version end)
   end
 
   defp lock_for_migrations(repo, opts, fun) do
@@ -268,6 +263,34 @@ defmodule Ecto.Migrator do
     |> Enum.reverse
   end
 
+  defp collect_migrations(versions, migration_source) do
+    ups_with_file =
+      versions
+      |> pending_in_direction(migration_source, :down)
+      |> Enum.map(fn {version, name, _} -> {:up, version, name} end)
+
+    ups_without_file =
+      versions
+      |> versions_without_file(migration_source)
+      |> Enum.map(fn version -> {:up, version, "** FILE NOT FOUND **"} end)
+
+    downs =
+      versions
+      |> pending_in_direction(migration_source, :up)
+      |> Enum.map(fn {version, name, _} -> {:down, version, name} end)
+
+    ups_with_file ++ ups_without_file ++ downs
+  end
+
+  defp versions_without_file(versions, migration_source) do
+    versions_with_file =
+      migration_source
+      |> migrations_for
+      |> Enum.map(&elem(&1, 0))
+
+    versions -- versions_with_file
+  end
+
   # This function will match directories passed into `Migrator.run`.
   defp migrations_for(migration_source) when is_binary(migration_source) do
     query = Path.join(migration_source, "*")
diff --git a/lib/ecto/query/builder.ex b/lib/ecto/query/builder.ex
index c4e70728..1e9abf52 100644
--- a/lib/ecto/query/builder.ex
+++ b/lib/ecto/query/builder.ex
@@ -240,6 +240,32 @@ defmodule Ecto.Query.Builder do
            "Instead use boolean operators: `and`, `or`, and `not`"
   end
 
+  # Tuple
+  def escape({left, right}, type, params_acc, vars, env) do
+    escape({:{}, [], [left, right]}, type, params_acc, vars, env)
+  end
+
+  # Tuple
+  def escape({:{}, _, list}, {:tuple, types}, params_acc, vars, env) do
+    if Enum.count(list) == Enum.count(types) do
+      {list, params_acc} =
+        list
+        |> Enum.zip(types)
+        |> Enum.map_reduce(params_acc, fn {expr, type}, params_acc ->
+             escape(expr, type, params_acc, vars, env)
+           end)
+      expr = {:{}, [], [:{}, [], list]}
+      {expr, params_acc}
+    else
+      escape({:{}, [], list}, :any, params_acc, vars, env)
+    end
+  end
+
+  # Tuple
+  def escape({:{}, _, _}, _, _, _, _) do
+    error! "Tuples can only be used in comparisons with literal tuples of the same size"
+  end
+
   # Other functions - no type casting
   def escape({name, _, args} = expr, type, params_acc, vars, env) when is_atom(name) and is_list(args) do
     case call_type(name, length(args)) do
@@ -677,6 +703,10 @@ defmodule Ecto.Query.Builder do
   def quoted_type(literal, _vars) when is_boolean(literal), do: :boolean
   def quoted_type(literal, _vars) when is_integer(literal), do: :integer
 
+  # Tuples
+  def quoted_type({left, right}, vars), do: quoted_type({:{}, [], [left, right]}, vars)
+  def quoted_type({:{}, _, elems}, vars), do: {:tuple, Enum.map(elems, &quoted_type(&1, vars))}
+
   def quoted_type({name, _, args}, _vars) when is_atom(name) and is_list(args) do
     case call_type(name, length(args)) do
       {_in, out} -> out


Test patch:
diff --git a/test/ecto/adapters/mysql_test.exs b/test/ecto/adapters/mysql_test.exs
index 91468c84..e98c9674 100644
--- a/test/ecto/adapters/mysql_test.exs
+++ b/test/ecto/adapters/mysql_test.exs
@@ -128,6 +128,9 @@ defmodule Ecto.Adapters.MySQLTest do
   test "where" do
     query = Schema |> where([r], r.x == 42) |> where([r], r.y != 43) |> select([r], r.x) |> normalize
     assert all(query) == ~s{SELECT s0.`x` FROM `schema` AS s0 WHERE (s0.`x` = 42) AND (s0.`y` != 43)}
+
+    query = Schema |> where([r], {r.x, r.y} > {1, 2}) |> select([r], r.x) |> normalize
+    assert all(query) == ~s{SELECT s0.`x` FROM `schema` AS s0 WHERE ((s0.`x`,s0.`y`) > (1,2))}
   end
 
   test "or_where" do
diff --git a/test/ecto/adapters/postgres_test.exs b/test/ecto/adapters/postgres_test.exs
index 780eed3d..5cc1ea0f 100644
--- a/test/ecto/adapters/postgres_test.exs
+++ b/test/ecto/adapters/postgres_test.exs
@@ -150,6 +150,9 @@ defmodule Ecto.Adapters.PostgresTest do
   test "where" do
     query = Schema |> where([r], r.x == 42) |> where([r], r.y != 43) |> select([r], r.x) |> normalize
     assert all(query) == ~s{SELECT s0."x" FROM "schema" AS s0 WHERE (s0."x" = 42) AND (s0."y" != 43)}
+
+    query = Schema |> where([r], {r.x, r.y} > {1, 2}) |> select([r], r.x) |> normalize
+    assert all(query) == ~s{SELECT s0."x" FROM "schema" AS s0 WHERE ((s0."x",s0."y") > (1,2))}
   end
 
   test "or_where" do
diff --git a/test/ecto/migrator_test.exs b/test/ecto/migrator_test.exs
index 0c8e9bb5..4bb72072 100644
--- a/test/ecto/migrator_test.exs
+++ b/test/ecto/migrator_test.exs
@@ -275,7 +275,7 @@ defmodule Ecto.MigratorTest do
     end
   end
 
-  test "version migrations work iside directories" do
+  test "version migrations work inside directories" do
     in_tmp fn path ->
       File.mkdir_p!("foo")
       create_migration "foo/13_version_in_dir.exs"
@@ -303,6 +303,26 @@ defmodule Ecto.MigratorTest do
     end
   end
 
+  test "migrations will give the migration status while file is deleted" do
+    in_tmp fn path ->
+      create_migration "1_up_migration_1.exs"
+      create_migration "2_up_migration_2.exs"
+      create_migration "3_up_migration_3.exs"
+      create_migration "4_down_migration_1.exs"
+
+      File.rm("2_up_migration_2.exs")
+
+      expected_result = [
+        {:up, 1, "up_migration_1"},
+        {:up, 2, "** FILE NOT FOUND **"},
+        {:up, 3, "up_migration_3"},
+        {:down, 4, "down_migration_1"},
+      ]
+
+      assert migrations(TestRepo, path) == expected_result
+    end
+  end
+
   test "migrations run inside a transaction if the adapter supports ddl transactions" do
     capture_log fn ->
       put_test_adapter_config(supports_ddl_transaction?: true, test_process: self())
diff --git a/test/ecto/query/builder/filter_test.exs b/test/ecto/query/builder/filter_test.exs
index 6a944b29..4715a4e3 100644
--- a/test/ecto/query/builder/filter_test.exs
+++ b/test/ecto/query/builder/filter_test.exs
@@ -19,6 +19,10 @@ defmodule Ecto.Query.Builder.FilterTest do
       assert escape(:where, quote do [x: ^"foo", y: ^"bar"] end, 0, [x: 0], __ENV__) ===
              {Macro.escape(quote do &0.x == ^0 and &0.y == ^1 end),
               %{0 => {"foo", {0, :x}}, 1 => {"bar", {0, :y}}}}
+
+      assert escape(:where, quote do {x.x} == {^"foo"} end, 0, [x: 0], __ENV__) ===
+             {Macro.escape(quote do {&0.x} == {^0} end),
+              %{0 => {"foo", {0, :x}}}}
     end
 
     test "raises on invalid expressions" do
@@ -26,6 +30,11 @@ defmodule Ecto.Query.Builder.FilterTest do
                    ~r"expected a keyword list at compile time in where, got: `\[\{1, 2\}\]`", fn ->
         escape(:where, quote do [{1, 2}] end, 0, [], __ENV__)
       end
+
+      assert_raise Ecto.Query.CompileError,
+                   ~r"Tuples can only be used in comparisons with literal tuples of the same size", fn ->
+        escape(:where, quote do {1, 2} > ^foo end, 0, [], __ENV__)
+      end
     end
 
     test "raises on nils" do