BUG: Can't decode FLOAT/DOUBLE in TextQuery when the fractional portion of the number is 0
benkimpel opened this issue · 7 comments
Can't decode FLOAT/DOUBLE in TextQuery when the fractional portion of the number is 0.
SQL Setup
CREATE TABLE example (id INT, value DOUBLE);
INSERT INTO example
VALUES (1, 100.123),
(2, 100.0)
;
Query Code
{:ok, conn} = MyXQL.start_link(...)
# All good
MyXQL.query!(conn, "SELECT * FROM example WHERE id = 1;", [], query_type: :text)
# Fails
MyXQL.query!(conn, "SELECT * FROM example WHERE id = 2;", [], query_type: :text)
# ** (ArgumentError) errors were found at the given arguments:
# * 1st argument: not a textual representation of a float
#
# ...snip...
#
# :erlang.binary_to_float("100")
# (myxql 0.6.4) lib/myxql/protocol/values.ex:132: MyXQL.Protocol.Values.decode_text_value/2
Test Case
Here's a test case that reproduces the issue:
# test/myxql/protocol/values_test.exs
# ...
test "MYSQL_TYPE_FLOAT", c do
assert Float.round(insert_and_get(c, "my_float", 10.0), 2) == 10.0 # <== HERE
assert Float.round(insert_and_get(c, "my_float", -13.37), 2) == -13.37
assert Float.round(insert_and_get(c, "my_float", 13.37), 2) == 13.37
assert Float.round(insert_and_get(c, "my_unsigned_float", 13.37), 2) == 13.37
end
# ...
Potential Fix
# lib/myxql/protocol/values.ex
def decode_text_value(value, type) when type in [:float, :double] do
value = if String.contains(value, "."), do: value, else: "#{value}.0"
String.to_float(value)
end
I can open a pull request if you want.
I imagine this isn't a bigger problem because it doesn't seem to effect prepared statements and it seems like that is mostly (exclusively?) what Ecto does.
I imagine this isn't a bigger problem because it doesn't seem to effect prepared statements and it seems like that is mostly (exclusively?) what Ecto does.
That's correct.
def decode_text_value(value, type) when type in [:float, :double] do
value = if String.contains(value, "."), do: value, else: "#{value}.0"
String.to_float(value)
end
Maybe Float.parse
instead...it seems to take care of it automatically https://hexdocs.pm/elixir/1.11.0/Float.html#parse/1
Maybe
Float.parse
instead...it seems to take care of it automatically https://hexdocs.pm/elixir/1.11.0/Float.html#parse/1
Yeah, Float.parse
could work. I'm not sure how you'd prefer to handle errors in these functions so that would definitely be a factor in the fix.
case Float.parse(value) do
{value, _rest} -> value
:error -> ?
end
I think raising so it matches the old behaviour. But we can put a better message. Something that says we are trying to decode this value into a float and it's not a float.
We should also probably raise when rest is not empty string. That would be more in line with the old behaviour. i.e. String.to_float("123.xtz") raises
I think raising so it matches the old behaviour. But we can put a better message. Something that says we are trying to decode this value into a float and it's not a float.
We should also probably raise when rest is not empty string. That would be more in line with the old behaviour. i.e. String.to_float("123.xtz") raises
I like that. We can raise an ArgumentError
(what it previously raised) with a better message.
case Float.parse(value) do
{value, ""} -> value
_ -> raise ArgumentError, "cannot parse FLOAT/DOUBLE"
end
perfect !