Ecto: "Cannot load `#Decimal<..>` as type :integer" on coalesce in subquery

Created on 12 Dec 2019 · 4Comments · Source: elixir-ecto/ecto

Environment

Elixir version (elixir -v):

Erlang/OTP 22 [erts-10.5.5] [source] [64-bit] [smp:12:12] [ds:12:12:10] [async-threads:1] [hipe]

Elixir 1.9.4 (compiled with Erlang/OTP 20)

Database and version (PostgreSQL 9.4, MongoDB 3.2, etc.):

postgres (PostgreSQL) 11.2

Ecto version (mix deps):

* ecto: 3.3.0 8ce4a6983ff6ea55517596f45e617ff0999dfa4a
* ecto_sql 3.3.0 7c88e87a96003a4ba8508f711797b54aa2fd87dd

Operating system:

      System Version: macOS 10.14.4 (18E226)
      Kernel Version: Darwin 18.5.0

Current behavior

Clone both ecto and ecto_sql repos.
Add the following migration to ecto_sql/integration_test/support/migration.exs:

    create table(:cart_items) do
      add :quantity, :decimal, precision: 29, scale: 9
      add :price, :decimal, precision: 29, scale: 9
    end

Add the following schema to ecto/integration_test/support/schemas.exs:

defmodule Ecto.Integration.CartItem do
  use Ecto.Integration.Schema

  schema "cart_items" do
    field(:quantity, :decimal)
    field(:price, :decimal)
    field(:total, :decimal, virtual: true)
  end
end

Add the following test to ecto/integration_test/cases/repo.exs:

  alias Ecto.Integration.CartItem

  test "coalesce in subquery" do
    TestRepo.insert!(%CartItem{quantity: 2, price: 3})

    q =
      from(
        cart_item in CartItem,
        select: %{cart_item | total: coalesce(cart_item.quantity * cart_item.price, 0)}
      )

    cart_items = TestRepo.all(q)
    assert [_] = cart_items
    [cart_item] = cart_items
    assert Decimal.eq?(cart_item.total, 6)

    # This throws an error:
    from(x in subquery(q), select: x) |> TestRepo.all()
  end

Run tests from ecto_sql: ECTO_PATH=../ecto mix test.all

The new test fails on line from(x in subquery(q), select: x) |> TestRepo.all() due to this exception:

  1) test coalesce in subquery (Ecto.Integration.RepoTest)
     ** (ArgumentError) cannot load `#Decimal<6.000000000000000000>` as type :integer                                                                                                                                                              code: from(x in subquery(q), select: x) |> TestRepo.all()
     stacktrace:
       (ecto) lib/ecto/repo/queryable.ex:371: Ecto.Repo.Queryable.process/4
       (ecto) lib/ecto/repo/queryable.ex:356: anonymous fn/4 in Ecto.Repo.Queryable.process_kv/4
       (elixir) lib/enum.ex:1440: Enum."-map_reduce/3-lists^mapfoldl/2-0-"/3
       (elixir) lib/enum.ex:1440: Enum."-map_reduce/3-lists^mapfoldl/2-0-"/3
       (ecto) lib/ecto/repo/queryable.ex:297: Ecto.Repo.Queryable.process/4
       (ecto) lib/ecto/repo/queryable.ex:205: anonymous fn/4 in Ecto.Repo.Queryable.preprocessor/3
       (elixir) lib/enum.ex:1336: Enum."-map/2-lists^map/1-0-"/2
       (ecto) lib/ecto/repo/queryable.ex:189: Ecto.Repo.Queryable.execute/4
       (ecto) lib/ecto/repo/queryable.ex:17: Ecto.Repo.Queryable.all/3
       /Users/jordi/dev/ecto/integration_test/cases/repo.exs:23: (test)

Expected behavior

I expected the behaviour we had in previous ecto version (the test above passes when run against ecto 3.2.0 and ecto_sql 3.2.2).

I'm not familiar with Ecto's codebase, but it seems that the changes done to Query.Planner.collect_fields in commit https://github.com/elixir-ecto/ecto/commit/4bd6b4045a415b67268212be2bab838036564545 caused this regression.

Prior to this commit, a number literal (i.e. 0) would be mapped by collect_fields to its own literal value:
https://github.com/elixir-ecto/ecto/blob/0ccb924ac52caebd6936de92302239e08889d116/lib/ecto/query/planner.ex#L1267-L1269

This would cause the :coalesce clause in collect_fields to return a {:value, :any} type for the coalesce call in the test above:
https://github.com/elixir-ecto/ecto/blob/0ccb924ac52caebd6936de92302239e08889d116/lib/ecto/query/planner.ex#L1177-L1189

Commit https://github.com/elixir-ecto/ecto/commit/4bd6b4045a415b67268212be2bab838036564545 changed that behaviour so now an integer literal gets mapped to {:value, :integer} by collect_fields:
https://github.com/elixir-ecto/ecto/blob/8ce4a6983ff6ea55517596f45e617ff0999dfa4a/lib/ecto/query/planner.ex#L1263-L1265

This causes the :coalesce clause in collect_fields to decide that the return type of the coalesce call in the test above will be that of the literal.

I don't know why this problem only occurs when the coalesce is ran in a subquery though.

If one can't determine the type of the first argument of a coalesce expression, as seems to be the case in my test above, it seems unsafe to assume that the returning type of that whole coalesce expression will be the type of the second argument. Therefore, tweaking the :coalesce clause might be a solution to this problem:

 defp collect_fields({:coalesce, _, [left, _right]} = expr, fields, from, query, take) do 
   {left_type, _, _} = collect_fields(left, fields, from, query, take) 

   type = 
     case left_type do 
       {:value, type} -> {:value, type} 
       _ -> {:value, :any} 
     end 

   {type, [expr | fields], from} 
 end

All tests pass with this change.

Source