From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@dev.tarantool.org, kostja.osipov@gmail.com,
imun@tarantool.org, korablev@tarantool.org
Subject: [Tarantool-patches] [PATCH v2 1/4] tuple: don't truncate float in :update()
Date: Tue, 4 Feb 2020 23:53:44 +0100 [thread overview]
Message-ID: <1a28ebaa58b81f4cbc3a1656cf364f281919ade9.1580856721.git.v.shpilevoy@tarantool.org> (raw)
In-Reply-To: <cover.1580856721.git.v.shpilevoy@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
---
src/box/xrow_update_field.c | 10 ++++--
test/box/update.result | 65 +++++++++++++++++++++++++++++++++++++
test/box/update.test.lua | 45 +++++++++++++++++++++++++
3 files changed, 117 insertions(+), 3 deletions(-)
diff --git a/src/box/xrow_update_field.c b/src/box/xrow_update_field.c
index 7c0f5fb5e..6ac29de5d 100644
--- a/src/box/xrow_update_field.c
+++ b/src/box/xrow_update_field.c
@@ -32,6 +32,8 @@
#include "tuple_format.h"
#include "mp_extension_types.h"
+#include <float.h>
+
/* {{{ Error helpers. */
/** Take a string identifier of a field being updated by @a op. */
@@ -400,13 +402,15 @@ xrow_update_arith_make(struct xrow_update_op *op,
unreachable();
break;
}
- if (lowest_type == XUPDATE_TYPE_DOUBLE) {
+ float fc = (float) c;
+ if ((lowest_type == XUPDATE_TYPE_DOUBLE && c != (double) fc) ||
+ (lowest_type == XUPDATE_TYPE_FLOAT &&
+ (c >= FLT_MAX || c <= -FLT_MAX))) {
ret->type = XUPDATE_TYPE_DOUBLE;
ret->dbl = c;
} else {
- assert(lowest_type == XUPDATE_TYPE_FLOAT);
ret->type = XUPDATE_TYPE_FLOAT;
- ret->flt = (float)c;
+ ret->flt = fc;
}
} else {
decimal_t a, b, c;
diff --git a/test/box/update.result b/test/box/update.result
index 28ba47831..dfbd8714f 100644
--- a/test/box/update.result
+++ b/test/box/update.result
@@ -1647,3 +1647,68 @@ t:update({{'=', '[2].d.g', 6000}, {'#', '[2].d.old.old', 1}})
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 314ebef05..74f9c62e2 100644
--- a/test/box/update.test.lua
+++ b/test/box/update.test.lua
@@ -597,3 +597,48 @@ t:update({{'=', '[2].d.g', 6000}, {'=', '[2].d.new.new', -1}})
t:update({{'=', '[2].d.g', 6000}, {'#', '[2].d.old.old', 1}})
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)
next prev parent reply other threads:[~2020-02-04 22:53 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-04 22:53 [Tarantool-patches] [PATCH v2 0/4] Don't truncate float in update Vladislav Shpilevoy
2020-02-04 22:53 ` Vladislav Shpilevoy [this message]
2020-02-10 14:41 ` [Tarantool-patches] [PATCH v2 1/4] tuple: don't truncate float in :update() Nikita Pettik
2020-02-10 21:18 ` Vladislav Shpilevoy
2020-02-19 21:36 ` Nikita Pettik
2020-02-04 22:53 ` [Tarantool-patches] [PATCH v2 2/4] tuple: pass tuple format to xrow_update_*_store() Vladislav Shpilevoy
2020-02-10 16:51 ` Nikita Pettik
2020-02-10 21:18 ` Vladislav Shpilevoy
2020-02-04 22:53 ` [Tarantool-patches] [PATCH v2 3/4] tuple: allow xrow_update sizeof reserve more memory Vladislav Shpilevoy
2020-02-10 16:05 ` Nikita Pettik
2020-02-04 22:53 ` [Tarantool-patches] [PATCH v2 4/4] tuple: use field type in update of a float/double Vladislav Shpilevoy
2020-02-10 16:16 ` Nikita Pettik
2020-02-10 21:18 ` Vladislav Shpilevoy
2020-02-05 11:09 ` [Tarantool-patches] [PATCH v2 0/4] Don't truncate float in update Konstantin Osipov
2020-02-05 22:11 ` Vladislav Shpilevoy
2020-02-20 6:15 ` Kirill Yukhin
2020-02-20 13:27 ` Nikita Pettik
2020-02-20 20:30 ` Vladislav Shpilevoy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1a28ebaa58b81f4cbc3a1656cf364f281919ade9.1580856721.git.v.shpilevoy@tarantool.org \
--to=v.shpilevoy@tarantool.org \
--cc=imun@tarantool.org \
--cc=korablev@tarantool.org \
--cc=kostja.osipov@gmail.com \
--cc=tarantool-patches@dev.tarantool.org \
--subject='Re: [Tarantool-patches] [PATCH v2 1/4] tuple: don'\''t truncate float in :update()' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox