From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp59.i.mail.ru (smtp59.i.mail.ru [217.69.128.39]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 9FCC3469719 for ; Sun, 23 Feb 2020 17:11:34 +0300 (MSK) From: Vladislav Shpilevoy Date: Sun, 23 Feb 2020 15:11:31 +0100 Message-Id: <2b1afed1b05757c177b2fbb2efbf7643670c43cc.1582466958.git.v.shpilevoy@tarantool.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH 1/1] tuple: don't truncate float in :update() List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org, kyukhin@tarantool.org Before the patch there were the rules: * float +/- double = double * double +/- double = double * float +/- float = float The rules were applied regardless of values. That led to a problem when float + float exceeding maximal float value could fit into double, but was stored as an infinity. The patch makes so that if a floating point arithmetic operation result fits into float, it is stored as float. Otherwise as double. Regardless of initial types. This alongside saves some memory for cases when doubles can be stored as floats, and therefore takes 4 less bytes. Although these cases are rare, because any not integer value stored in a double may have a long garbage tail in its fraction. Closes #4701 (cherry picked from commit fef4fdfc3edee39c72f2e93e6f96ab681750617f) --- Branch: https://github.com/tarantool/tarantool/tree/gerold103/gh-4701-update-float-2.2 Issue: https://github.com/tarantool/tarantool/issues/4701 Fix for 4701 was not pushed to 2.2 and 1.10. But it is a bug fix, and it should be pushed to them too. This branch is for 2.2 and it should be easy to cherry-pick it to 1.10. @ChangeLog - tuple/space/index:update()/upsert() were fixed not to turn a value into an infinity, when a float value was added to or subtracted from a float column and exceeded float value range (gh-4701). src/box/tuple_update.c | 11 +++---- test/box/update.result | 65 ++++++++++++++++++++++++++++++++++++++++ test/box/update.test.lua | 45 ++++++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 5 deletions(-) diff --git a/src/box/tuple_update.c b/src/box/tuple_update.c index 7a203ced8..785a88640 100644 --- a/src/box/tuple_update.c +++ b/src/box/tuple_update.c @@ -33,6 +33,7 @@ #include #include +#include #include "say.h" #include "error.h" @@ -484,15 +485,15 @@ make_arith_operation(struct op_arith_arg arg1, struct op_arith_arg arg2, err_fieldno, "a positive integer"); return -1; } - if (lowest_type == AT_DOUBLE) { - /* result is DOUBLE */ + float fc = (float)c; + if ((lowest_type == AT_DOUBLE && c != (double) fc) || + (lowest_type == AT_FLOAT && + (c >= FLT_MAX || c <= -FLT_MAX))) { ret->type = AT_DOUBLE; ret->dbl = c; } else { - /* result is FLOAT */ - assert(lowest_type == AT_FLOAT); ret->type = AT_FLOAT; - ret->flt = (float)c; + ret->flt = fc; } } return 0; diff --git a/test/box/update.result b/test/box/update.result index a3f731b55..0dbe0f0a1 100644 --- a/test/box/update.result +++ b/test/box/update.result @@ -849,3 +849,68 @@ s:update(1, {{'=', 3, map}}) s:drop() --- ... +-- +-- gh-4701: arith operations should not truncate result when +-- float + float fits double. +-- +msgpackffi = require('msgpackffi') +--- +... +mp_array_1 = 0x91 +--- +... +mp_double = 0xcb +--- +... +mp_float = 0xca +--- +... +flt_max = 3.402823466e+38 +--- +... +uint_max = 18446744073709551615ULL +--- +... +-- Double + double is float if result fits. \ +-- Double + double is double if result does not fit float. \ +-- Double + float is float if result fits. \ +-- Double + float is double if result does not fit float. \ +-- Float + float is float when no overflow. \ +-- Float + float is double when overflow. \ +-- Float + int is float when fits the float range. \ +-- Precision matters too. Double is used when need to avoid \ +-- precision loss. \ +tests = { \ + {{'double', 1}, {'double', 1}, mp_float}, \ + {{'double', flt_max}, {'double', flt_max}, mp_double}, \ + {{'double', 1}, {'float', 1}, mp_float}, \ + {{'double', flt_max}, {'float', flt_max}, mp_double}, \ + {{'float', 1}, {'float', 1}, mp_float}, \ + {{'float', flt_max}, {'float', flt_max}, mp_double}, \ + {{'float', -flt_max}, {'float', -flt_max}, mp_double}, \ + {{'float', 1}, {'int', 1}, mp_float}, \ + {{'float', flt_max}, {'uint64_t', uint_max}, mp_double}, \ + {{'float', 1.0001}, {'double', 1.0000000000001}, mp_double}, \ +} +--- +... +err = nil +--- +... +for i, test in pairs(tests) do \ + local val1 = ffi.cast(test[1][1], test[1][2]) \ + local val2 = ffi.cast(test[2][1], test[2][2]) \ + local t = box.tuple.new({val1}) \ + t = t:update({{'+', 1, val2}}) \ + local m = msgpackffi.encode(t) \ + if m:byte(1) ~= mp_array_1 or m:byte(2) ~= test[3] then \ + err = {i, test, t, m:byte(1), m:byte(2)} \ + break \ + end \ +end +--- +... +err +--- +- null +... diff --git a/test/box/update.test.lua b/test/box/update.test.lua index c455bc1e1..724011c38 100644 --- a/test/box/update.test.lua +++ b/test/box/update.test.lua @@ -269,3 +269,48 @@ t:update({{'=', 3, map}}) s:update(1, {{'=', 3, map}}) s:drop() + +-- +-- gh-4701: arith operations should not truncate result when +-- float + float fits double. +-- +msgpackffi = require('msgpackffi') +mp_array_1 = 0x91 +mp_double = 0xcb +mp_float = 0xca +flt_max = 3.402823466e+38 +uint_max = 18446744073709551615ULL +tests = { \ +-- Double + double is float if result fits. \ + {{'double', 1}, {'double', 1}, mp_float}, \ +-- Double + double is double if result does not fit float. \ + {{'double', flt_max}, {'double', flt_max}, mp_double}, \ +-- Double + float is float if result fits. \ + {{'double', 1}, {'float', 1}, mp_float}, \ +-- Double + float is double if result does not fit float. \ + {{'double', flt_max}, {'float', flt_max}, mp_double}, \ +-- Float + float is float when no overflow. \ + {{'float', 1}, {'float', 1}, mp_float}, \ +-- Float + float is double when overflow. \ + {{'float', flt_max}, {'float', flt_max}, mp_double}, \ + {{'float', -flt_max}, {'float', -flt_max}, mp_double}, \ +-- Float + int is float when fits the float range. \ + {{'float', 1}, {'int', 1}, mp_float}, \ + {{'float', flt_max}, {'uint64_t', uint_max}, mp_double}, \ +-- Precision matters too. Double is used when need to avoid \ +-- precision loss. \ + {{'float', 1.0001}, {'double', 1.0000000000001}, mp_double}, \ +} +err = nil +for i, test in pairs(tests) do \ + local val1 = ffi.cast(test[1][1], test[1][2]) \ + local val2 = ffi.cast(test[2][1], test[2][2]) \ + local t = box.tuple.new({val1}) \ + t = t:update({{'+', 1, val2}}) \ + local m = msgpackffi.encode(t) \ + if m:byte(1) ~= mp_array_1 or m:byte(2) ~= test[3] then \ + err = {i, test, t, m:byte(1), m:byte(2)} \ + break \ + end \ +end +err -- 2.21.1 (Apple Git-122.3)