Repository: elixir-ecto/ecto_sql
Pull request #639 (2024-10-07 12:05:01)

Clarify that index names can be a string or an atom
Definitely both work, I've tried it out to make sure. Not having it specified caused some confusion in my team and I think it can help to specify this.

I did change the time spec, as I'm rather sure it also ends up in there as a string.

I'm a bit surprised/confused that the docs default to atom usage as as best I can tell the index is used for instance here:

(Postgres connection)
```elixir
    queries = [
        [
          "CREATE ",
          if_do(index.unique, "UNIQUE "),
          "INDEX ",
          if_do(index.concurrently, "CONCURRENTLY "),
          if_do(command == :create_if_not_exists, "IF NOT EXISTS "),
          quote_name(index.name),
          " ON ",
          more stuff

```

which ends up converting the atom back to a string:

```elixir
    defp quote_name(nil, name), do: quote_name(name)

    defp quote_name(prefix, name), do: [quote_name(prefix), ?., quote_name(name)]

    defp quote_name(name) when is_atom(name) do
      quote_name(Atom.to_string(name))
    end

    more stuff
```

But yeah, I don't know as well - I was thinking maybe to use name as a string in the docs at least once to show it's possible but maybe that breaks with a desire for uniformity.

As always, thank you all for your wonderful work! :green_heart: 


Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index b63ce29..49315f4 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -58,6 +58,8 @@ jobs:
           - "11.11-alpine"
           - "9.6-alpine"
           - "9.5-alpine"
+        pool_count:
+          - "1"
         include:
           - elixirbase: "1.11.4-erlang-23.3.4.9-alpine-3.16.9"
             postgres: "16.2-alpine"
diff --git a/lib/ecto/adapters/myxql/connection.ex b/lib/ecto/adapters/myxql/connection.ex
index cc951c5..eb172a7 100644
--- a/lib/ecto/adapters/myxql/connection.ex
+++ b/lib/ecto/adapters/myxql/connection.ex
@@ -826,7 +826,7 @@ if Code.ensure_loaded?(MyXQL) do
             fun,
             ?(,
             modifier,
-            Enum.map_intersperse(args, ", ", &top_level_expr(&1, sources, query)),
+            Enum.map_intersperse(args, ", ", &expr(&1, sources, query)),
             ?)
           ]
       end
@@ -881,7 +881,7 @@ if Code.ensure_loaded?(MyXQL) do
     end
 
     defp values_list(types, num_rows, query) do
-      rows = Enum.to_list(1..num_rows)
+      rows = :lists.seq(1, num_rows, 1)
 
       [
         "VALUES ",
diff --git a/lib/ecto/adapters/postgres/connection.ex b/lib/ecto/adapters/postgres/connection.ex
index 42f3d5a..996e8e1 100644
--- a/lib/ecto/adapters/postgres/connection.ex
+++ b/lib/ecto/adapters/postgres/connection.ex
@@ -1137,7 +1137,7 @@ if Code.ensure_loaded?(Postgrex) do
     end
 
     defp values_list(types, idx, num_rows) do
-      rows = Enum.to_list(1..num_rows)
+      rows = :lists.seq(1, num_rows, 1)
 
       [
         "VALUES ",
diff --git a/lib/ecto/adapters/tds/connection.ex b/lib/ecto/adapters/tds/connection.ex
index 2323520..9eeeb0b 100644
--- a/lib/ecto/adapters/tds/connection.ex
+++ b/lib/ecto/adapters/tds/connection.ex
@@ -981,7 +981,7 @@ if Code.ensure_loaded?(Tds) do
     end
 
     defp values_list(types, idx, num_rows) do
-      rows = Enum.to_list(1..num_rows)
+      rows = :lists.seq(1, num_rows, 1)
 
       [
         "VALUES ",
diff --git a/lib/ecto/migration.ex b/lib/ecto/migration.ex
index c1f28e1..8b0d819 100644
--- a/lib/ecto/migration.ex
+++ b/lib/ecto/migration.ex
@@ -396,7 +396,7 @@ defmodule Ecto.Migration do
     @type t :: %__MODULE__{
             table: String.t(),
             prefix: String.t() | nil,
-            name: atom,
+            name: String.t() | atom,
             columns: [atom | String.t()],
             unique: boolean,
             concurrently: boolean,
@@ -779,7 +779,8 @@ defmodule Ecto.Migration do
 
   ## Options
 
-    * `:name` - the name of the index. Defaults to "#{table}_#{column}_index".
+    * `:name` - the name of the index. Can be provided as a string or an atom.
+    Defaults to "#{table}_#{column}_index".
     * `:prefix` - specify an optional prefix for the index.
     * `:unique` - indicates whether the index should be unique. Defaults to `false`.
     * `:comment` - adds a comment to the index.


Test patch:
diff --git a/test/ecto/adapters/myxql_test.exs b/test/ecto/adapters/myxql_test.exs
index c2d499d..ee02cbb 100644
--- a/test/ecto/adapters/myxql_test.exs
+++ b/test/ecto/adapters/myxql_test.exs
@@ -416,6 +416,14 @@ defmodule Ecto.Adapters.MyXQLTest do
     assert all(query) == ~s{SELECT coalesce(s0.`x`, 5) FROM `schema` AS s0}
   end
 
+  test "coalesce with subquery" do
+    squery = from s in Schema, select: s.x
+    query = Schema |> select([s], coalesce(subquery(squery), 5)) |> plan()
+
+    assert all(query) ==
+             ~s{SELECT coalesce((SELECT ss0.`x` AS `x` FROM `schema` AS ss0), 5) FROM `schema` AS s0}
+  end
+
   test "where" do
     query = Schema |> where([r], r.x == 42) |> where([r], r.y != 43) |> select([r], r.x) |> plan()
 
@@ -478,7 +486,7 @@ defmodule Ecto.Adapters.MyXQLTest do
       |> plan()
 
     assert all(query) ==
-             ~s{SELECT s0.`x` FROM `schema` AS s0 ORDER BY exists(SELECT ss0.`x` AS `result` FROM `schema` AS ss0 WHERE (ss0.`x` = s0.`x`))}
+             ~s{SELECT s0.`x` FROM `schema` AS s0 ORDER BY exists((SELECT ss0.`x` AS `result` FROM `schema` AS ss0 WHERE (ss0.`x` = s0.`x`)))}
   end
 
   test "union and union all" do
@@ -882,7 +890,7 @@ defmodule Ecto.Adapters.MyXQLTest do
       |> plan()
 
     assert all(query) ==
-             ~s{SELECT s0.`x` FROM `schema` AS s0 GROUP BY exists(SELECT ss0.`x` AS `result` FROM `schema` AS ss0 WHERE (ss0.`x` = s0.`x`))}
+             ~s{SELECT s0.`x` FROM `schema` AS s0 GROUP BY exists((SELECT ss0.`x` AS `result` FROM `schema` AS ss0 WHERE (ss0.`x` = s0.`x`)))}
   end
 
   test "interpolated values" do
@@ -1089,7 +1097,7 @@ defmodule Ecto.Adapters.MyXQLTest do
         |> plan
 
       assert all(query) ==
-               ~s{SELECT s0.`x` FROM `schema` AS s0 WINDOW `w` AS (ORDER BY exists(SELECT ss0.`x` AS `result` FROM `schema` AS ss0 WHERE (ss0.`x` = s0.`x`)))}
+               ~s{SELECT s0.`x` FROM `schema` AS s0 WINDOW `w` AS (ORDER BY exists((SELECT ss0.`x` AS `result` FROM `schema` AS ss0 WHERE (ss0.`x` = s0.`x`))))}
     end
 
     test "two windows" do
diff --git a/test/ecto/adapters/postgres_test.exs b/test/ecto/adapters/postgres_test.exs
index 617f59f..e9a64cf 100644
--- a/test/ecto/adapters/postgres_test.exs
+++ b/test/ecto/adapters/postgres_test.exs
@@ -584,6 +584,14 @@ defmodule Ecto.Adapters.PostgresTest do
     assert all(query) == ~s{SELECT coalesce(s0."x", 5) FROM "schema" AS s0}
   end
 
+  test "coalesce with subquery" do
+    squery = from s in Schema, select: s.x
+    query = Schema |> select([s], coalesce(subquery(squery), 5)) |> plan()
+
+    assert all(query) ==
+             ~s{SELECT coalesce((SELECT ss0."x" AS "x" FROM "schema" AS ss0), 5) FROM "schema" AS s0}
+  end
+
   test "where" do
     query = Schema |> where([r], r.x == 42) |> where([r], r.y != 43) |> select([r], r.x) |> plan()