phoenixframework / phoenix

Peace of mind from prototype to production
https://www.phoenixframework.org
MIT License
21.4k stars 2.87k forks source link

Decimal support #1347

Closed dbulic closed 8 years ago

dbulic commented 8 years ago

Working with Postgres database, I use numeric fields all over the place. As I understand, decimal is not a native Elixir/Erlang type and it doesn't seem to support any basic arithmetic (tried + and -). Here is a small example mode, where I want to set up a calculated field and had to resort to manual conversion of each decimal to float. I could probably create a Decimal module myself, but wanted to check first if I am missing something obvious. We do need decimals to work out of the box.

defmodule Phoenixcore.Order do
  use Ecto.Model

  after_load :calc

  schema "orders" do
    field :ordered_on, Ecto.DateTime
    field :licno, :string
    field :description, :string
    field :total_usd, :decimal
    field :tax_usd, :decimal
    field :discount_usd, :decimal
    field :revenue_usd, :decimal, virtual: true
    belongs_to :user, Phoenixcore.User

    timestamps([type: Ecto.DateTime, usec: false, inserted_at: :created_at, updated_at: :updated_at])
  end

  def calc(model) do
    {total, _ } = Float.parse(Decimal.to_string(model.total_usd))
    {discount, _} = Float.parse(Decimal.to_string(model.discount_usd))
    {tax, _} = Float.parse(Decimal.to_string(model.tax_usd))
    Map.put(model, :revenue_usd, total - discount - tax)
  end

end
josevalim commented 8 years ago

Why are you not using the functions in the Decimal module? Doing operations on float is usually a bad idea due to precision. Docs for decimal can be found here: http://hexdocs.pm/decimal

dbulic commented 8 years ago

Couldn't find the docs (I have just started with Elixir and Phoenix) for decimal. Thanks.

dbulic commented 8 years ago

It's easy to use standard Decimal functions, but they are pretty unreadable for what is a very basic operation: Decimal.sub(Decimal.sub(model.total_usd, model.discount_usd), model.tax_usd)

Is there a way to add standard infix notation to Decimal type?

dbulic commented 8 years ago

I understand Decimal is a package. What I am asking here is can infix operators be added to a package at all. Not sure if Elixir allows that. If so, I would appreciate pointers to docs, so I can add infix operators to Decimal package and issue a PR to @ericmj .

chrismccord commented 8 years ago
import Decimal

model.total_usd
|> sub(model.discount_usd)
|> sub(model.tax_usd)
dbulic commented 8 years ago

Thanks @chrismccord - it certainly helps a lot. I would really like to help to make Decimal feel more like a built-in type, but I don't know what can be done. Don't want to be a PITA here, just looking to help if I may (and make my own project simpler).

Here is another limitation:

    q = from o in Order, where: o.user_id == ^model.id, select: o.total_usd
    Repo.all(q) |> Enum.reduce(Decimal.new(0), fn(x,acc) -> Decimal.add(x,acc) end)

What needs to be done to have:

    q = from o in Order, where: o.user_id == ^model.id, select: o.total_usd
    Repo.all(q) |> Enum.sum

Again, I volunteer to do the job - just want to be sure it can be done at all.

josevalim commented 8 years ago

It is not an easy task at the moment. Things will hopefully get easier when Decimal gets merged into core. We can either go with your take:

q = from o in Order, where: o.user_id == ^model.id, select: o.total_usd
Repo.all(q) |> Enum.reduce(Decimal.new(0), &Decimal.add/2)

Or do the sum on Ecto:

q = from o in Order, where: o.user_id == ^model.id, select: sum(o.total_usd)
total = Repo.one!(q)
dbulic commented 8 years ago

I believe both should be supported once Decimal is merged into core. People will need to work outside Ecto with decimal. In-query sums simplify working with queries as extra step is avoided (and it nicely maps to underlying SQL construct).

Thanks, will do my best to start contributing once I get more experience with Elixir.

SebastianSzturo commented 5 years ago

Any updates on Decimal being added to core and supporting Enum.sum?

ericmj commented 5 years ago

There are currently no plans to add Decimal to core.