Browse Source

Float rounding; see #4

Viktor Söderqvist 10 years ago
parent
commit
308776c475
3 changed files with 90 additions and 26 deletions
  1. 1 1
      src/mysql.erl
  2. 38 18
      src/mysql_protocol.erl
  3. 51 7
      test/mysql_tests.erl

+ 1 - 1
src/mysql.erl

@@ -20,7 +20,7 @@
 -export([connect/1, disconnect/1, query/2, query/3, prepare/2, warning_count/1,
          affected_rows/1, insert_id/1]).
 
-%% @doc A MySQL error with the codes and message returned from the server.
+%% MySQL error with the codes and message returned from the server.
 -type reason() :: {Code :: integer(), SQLState :: binary(),
                    Message :: binary()}.
 

+ 38 - 18
src/mysql_protocol.erl

@@ -513,11 +513,12 @@ decode_binary(?TYPE_FLOAT, <<Value:32/float-little, Rest/binary>>) ->
     %% There is a precision loss when storing and fetching a 32-bit float.
     %% In the text protocol, it is obviously rounded. Storing 3.14 in a FLOAT
     %% column and fetching it using the text protocol, we get "3.14" which we
-    %% parse to the Erlang double 3.14. Fetching the same value as a binary
-    %% 32-bit float, we get 3.140000104904175. To achieve the same rounding
-    %% after receiving it as a 32-bit float, we try to do the same rounding here
-    %% as MySQL does before sending it over the text protocol. Here is a quote
-    %% of a comment in the documentation:
+    %% parse to the Erlang double as close as possible to 3.14. Fetching the
+    %% same value as a binary 32-bit float, we get 3.140000104904175. To achieve
+    %% the same rounding after receiving it as a 32-bit float, we try to do the
+    %% same rounding here as MySQL does when sending it over the text protocol.
+    %%
+    %% This comment explains the idea:
     %%
     %%     Posted by Geoffrey Downs on March 10 2011 10:26am
     %%
@@ -535,17 +536,14 @@ decode_binary(?TYPE_FLOAT, <<Value:32/float-little, Rest/binary>>) ->
     %% (From http://dev.mysql.com/doc/refman/5.7/en/problems-with-float.html
     %% fetched 10 Nov 2014)
     %%
-    %Precision = math:pow(10, -5 + trunc(math:log10(abs(Value)))),
-    %% Round to this precision
-    %InvPrec = 1 / Precision,
-    %RoundedValue = round(InvPrec * Value) / InvPrec,
-    %% Note: If we multiply be Precision after rounding instead of dividing by
-    %% InvPrec, we get rouding errors.
-    %{RoundedValue, Rest};
-    %%---------- We don't use the above method as it gives us 3.1400000000000006
-    %%---------- for 3.14 (INSERT + SELECT roundtrip). This needs some tweaks
-    %%---------- and extensive testing with various numbers.
-    {Value, Rest};
+    %% The above is almost correct, except for the example in the interval
+    %% 0 < x < 1. There are 6 significant digits also for these numbers.
+    %%
+    %% Now, instead of P = 0.00001 we want the inverse 100000.0 but if we
+    %% compute Factor = 1 / P we get a precision loss, so instead we do this:
+    Factor = math:pow(10, floor(6 - math:log10(abs(Value)))),
+    RoundedValue = round(Value * Factor) / Factor,
+    {RoundedValue, Rest};
 decode_binary(?TYPE_DATE, <<Length, Data/binary>>) ->
     %% Coded in the same way as DATETIME and TIMESTAMP below, but returned in
     %% a simple triple.
@@ -590,11 +588,19 @@ decode_binary(?TYPE_TIME, <<Length, Data/binary>>) ->
             {{time, {-(D * 24 + H), -M, -S - 0.000001 * Micro}}, Rest}
     end.
 
+%% @doc Like trunc/1 but towards negative infinity instead of towards zero.
+floor(Value) ->
+    Trunc = trunc(Value),
+    if
+        Trunc =< Value -> Trunc;
+        Trunc > Value -> Trunc - 1 %% for negative values
+    end.
+
 %% @doc Encodes a term reprenting av value as a binary for use in the binary
 %% protocol. As this is used to encode parameters for prepared statements, the
-%% encoding is in its required form, namely <<Type:8, Sign:8, Value/binary>>.
+%% encoding is in its required form, namely `<<Type:8, Sign:8, Value/binary>>'.
 %%
-%% TODO: Maybe change Erlang representation of BIT to <<_:1>>.
+%% TODO: Maybe change Erlang representation of BIT to `<<_:1>>'.
 -spec encode_param(term()) -> {TypeAndSign :: binary(), Data :: binary()}.
 encode_param(null) ->
     {<<?TYPE_NULL, 0>>, <<>>};
@@ -883,6 +889,20 @@ decode_text_test() ->
     ?assertEqual(null, decode_text(?TYPE_FLOAT, null)),
     ok.
 
+decode_binary_test() ->
+    %% Test the special rounding we apply to (single precision) floats.
+    %?assertEqual({1.0, <<>>},
+    %             decode_binary(?TYPE_FLOAT, <<1.0:32/float-little>>)),
+    %?assertEqual({0.2, <<>>},
+    %             decode_binary(?TYPE_FLOAT, <<0.2:32/float-little>>)),
+    %?assertEqual({-33.3333, <<>>},
+    %             decode_binary(?TYPE_FLOAT, <<-33.333333:32/float-little>>)),
+    %?assertEqual({0.000123457, <<>>},
+    %             decode_binary(?TYPE_FLOAT, <<0.00012345678:32/float-little>>)),
+    %?assertEqual({1234.57, <<>>},
+    %             decode_binary(?TYPE_FLOAT, <<1234.56789:32/float-little>>)),
+    ok.
+
 null_bitmap_test() ->
     ?assertEqual({<<0, 1:1>>, <<>>}, null_bitmap_decode(9, <<0, 4>>, 2)),
     ?assertEqual(<<0, 4>>, null_bitmap_encode(<<0, 1:1>>, 2)),

+ 51 - 7
test/mysql_tests.erl

@@ -53,7 +53,8 @@ query_test_() ->
      end,
      {with, [fun basic_queries/1,
              fun text_protocol/1,
-             fun binary_protocol/1]}}.
+             fun binary_protocol/1,
+             fun float_rounding/1]}}.
 
 basic_queries(Pid) ->
 
@@ -112,16 +113,11 @@ binary_protocol(Pid) ->
     %% TODO: Put the expected result in a macro to make sure they are identical
     %% for the text and the binary protocol tests.
 
-    %% One thing that are not identical in the text and binary protocols are
-    %% floats. We get the float as a 32-bit float, so the value is not exactly
-    %% what we inserted.
-    <<ExpectedFloat:32/native-float>> = <<3.14:32/native-float>>,
-
     {ok, Stmt} = mysql:prepare(Pid, <<"SELECT * FROM t WHERE id=?">>),
     {ok, Columns, Rows} = mysql:query(Pid, Stmt, [1]),
     ?assertEqual([<<"id">>, <<"bl">>, <<"tx">>, <<"f">>, <<"dc">>, <<"ti">>,
                   <<"ts">>, <<"da">>, <<"c">>], Columns),
-    ?assertEqual([[1, <<"blob">>, <<>>, ExpectedFloat, <<"3.140">>,
+    ?assertEqual([[1, <<"blob">>, <<>>, 3.14, <<"3.140">>,
                    {time, {0, 22, 11}},
                    {{2014, 11, 03}, {00, 22, 24}}, {2014, 11, 03}, null]],
                  Rows),
@@ -135,3 +131,51 @@ binary_protocol(Pid) ->
     %% * Negative TIME
 
     ok = mysql:query(Pid, <<"DROP TABLE t">>).
+
+float_rounding(Pid) ->
+    %% This is to make sure we get the same values for 32-bit FLOATs in the text
+    %% and binary protocols for ordinary queries and prepared statements
+    %% respectively.
+    %%
+    %% MySQL rounds to 6 significant digits when "printing" floats over the
+    %% text protocol. When we receive a float on the binary protocol, we round
+    %% it in the same way to match what MySQL does on the text protocol. This
+    %% way we should to get the same values regardless of which protocol is
+    %% used.
+
+    %% Table for testing floats
+    ok = mysql:query(Pid, "CREATE TABLE f (f FLOAT)"),
+
+    %% Prepared statements
+    {ok, Insert} = mysql:prepare(Pid, "INSERT INTO f (f) VALUES (?)"),
+    {ok, Select} = mysql:prepare(Pid, "SELECT f FROM f"),
+
+    %% [{Input, Expected}]
+    TestData = [{1.0, 1.0}, {3.14, 3.14}, {0.2, 0.2},
+                {0.20082111, 0.200821}, {0.000123456789, 0.000123457},
+                {33.3333333, 33.3333}, {-33.2233443322, -33.2233},
+                {400.0123, 400.012}, {1000.1234, 1000.12},
+                {999.00009, 999.0},
+                {1234.5678, 1234.57}, {68888.8888, 68888.9},
+                {123456.789, 123457.0}, {7654321.0, 7654320.0},
+                {80001111.1, 80001100.0}, {987654321.0, 987654000.0},
+                {-123456789.0, -123457000.0},
+                {2.12345111e-23, 2.12345e-23}, {-2.12345111e-23, -2.12345e-23},
+                {2.12345111e23, 2.12345e23}, {-2.12345111e23, -2.12345e23}],
+    lists:foreach(fun ({Input, Expected}) ->
+                      %% Insert using binary protocol (sending it as a double)
+                      ok = mysql:query(Pid, Insert, [Input]),
+
+                      %% Text (plain query)
+                      {ok, _, [[Value]]} = mysql:query(Pid, "SELECT f FROM f"),
+                      ?assertEqual(Expected, Value),
+
+                      %% Binary (prepared statement)
+                      {ok, _, [[BinValue]]} = mysql:query(Pid, Select, []),
+                      ?assertEqual(Expected, BinValue),
+
+                      %% cleanup before the next test
+                      ok = mysql:query(Pid, "DELETE FROM f")
+                end,
+                TestData),
+    ok = mysql:query(Pid, "DROP TABLE f").