Ecto: Crash when validating NaN number

[lukaszsamson]

Environment

• Ecto version (mix deps): 2.10

Current behavior

When validating

MyStruct{}
|> cast(%{my_field: "NaN"})
|> validate_required([:my_field])
|> validate_number(: my_field, greater_than: 0, less_than: 100)

ecto crashes with

** (CaseClauseError) no case clause matching: #Decimal<NaN>
(decimal) lib/decimal.ex:349: Decimal.cmp/2
(ecto) lib/ecto/changeset.ex:1727: Ecto.Changeset.validate_number/6
(elixir) lib/enum.ex:2914: Enum.find_value_list/3
(ecto) lib/ecto/changeset.ex:1312: Ecto.Changeset.validate_change/3

Expected behavior

'validate_required' should check if the number is not NaN (just like it checks if string is not empty)
'validate_number' should not try to compare with NaN (or use Decimal.compare/2 instead of Decimal.cmp/2 and ignore NaN results)

[josevalim]

I don't think we should accept NaN. i.e. casting should fail.

I know some users may actually want NaN, but then they can leverage a custom type, especially once we add compare to the type API.

[josevalim]

Btw, thanks for the report! :)

[lukaszsamson]

I'm not sure if casting should fail. Some databases do support NaN values (e.g. Postgres, Oracle).
I worked around this issue with simple substitution - in my use case I can treat NaN as a missing value.

defp validate_not_nan(cs, field) do
    case get_field(cs, field) do
      %Decimal{coef: coef} when coef in [:sNaN, :qNaN] ->
        cs |> put_change(field, nil)
      _ ->
        cs
    end
  end

MyStruct{}
|> cast(%{my_field: "NaN"})
|> validate_not_nan(: my_field)
|> validate_required([:my_field])
|> validate_number(:my_field, greater_than: 0, less_than: 100)

[josevalim]

It should fail out of the box because I would say most users don't actually expect NaN in there. If you want NaN, then you can opt into it with a custom type.

[zachdaniel]

As long as the way NaN is handled is documented somewhere, along w/ the advice to use a custom type, I agree.

[lukaszsamson]

@josevalim @wojtekmach the patch does not fix the original issue.

MyStruct{}
|> cast(%{my_field: "NaN"}, ~w(my_field)a)
|> validate_required([:my_field])
|> validate_number(: my_field, greater_than: 0, less_than: 100)

now crashes with

** (Ecto.CastError) cannot cast "Infinity" to :decimal. Define custom type to handle NaN or infinite decimals
     code: changeset = cast(MyStruct{}, %{my_field: "NaN"}, ~w(my_field)a)
     stacktrace:
       (ecto) lib/ecto/type.ex:933: Ecto.Type.validate_decimal/2
       (ecto) lib/ecto/changeset.ex:532: Ecto.Changeset.cast_field/8
       (ecto) lib/ecto/changeset.ex:494: Ecto.Changeset.process_param/7
       (elixir) lib/enum.ex:1899: Enum."-reduce/3-lists^foldl/2-0-"/3
       (ecto) lib/ecto/changeset.ex:471: Ecto.Changeset.cast/6

IMO we should handle it like we handle decimal parse errors, i.e. set changeset error {"is invalid", [type: :decimal, validation: :cast]}

[wojtekmach]

I think you're right, cast(%{my_field: "NaN"}, ~w(my_field)a) probably shouldn't crash on user input like that. FWIW I was going for an approach that will raise an error with a message that will tell users what to do next but yeah. I'll send a patch this week.

[wojtekmach]

Fixed in #2585