[Tarantool-patches] [PATCH 05/11] xrow: don't cast double to float unconditionally

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Fri Jun 5 02:43:12 MSK 2020


Xrow update functions use function xrow_update_arith_make() to
execute arithmetic operation. In case any operand is a floating
point value, both are cast to double. Afterwards the result value
tries to shrink to float if possible, to save space.

The check whether the value fits float was done in a dumb but
simple way:

    (double)(float)value == value

This was done to see if the fractional part is not truncated. The
integral part was checked against FLT_MAX and -FLT_MAX. But the
latter check was done after the first check. That led to undefined
behaviour if the |value| is bigger than |FLT_MAX|. This patch
makes xrow update firstly check the integral bounds and then the
fractional part.

Alongside there is fixed a bug, when FLT_MAX value was considered
double.

Part of #4609
---
 src/box/xrow_update_field.c | 19 ++++++++++---------
 test/box/update.result      |  2 +-
 test/box/update.test.lua    |  2 +-
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/src/box/xrow_update_field.c b/src/box/xrow_update_field.c
index cc64cf955..d635e82b9 100644
--- a/src/box/xrow_update_field.c
+++ b/src/box/xrow_update_field.c
@@ -404,16 +404,17 @@ xrow_update_arith_make(struct xrow_update_op *op,
 			unreachable();
 			break;
 		}
-		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 {
-			ret->type = XUPDATE_TYPE_FLOAT;
-			ret->flt = fc;
+		if (c <= FLT_MAX && c >= -FLT_MAX) {
+			float fc = (float)c;
+			if (c == (double)fc) {
+				ret->type = XUPDATE_TYPE_FLOAT;
+				ret->flt = fc;
+				return 0;
+			}
 		}
+		ret->type = XUPDATE_TYPE_DOUBLE;
+		ret->dbl = c;
+		return 0;
 	} else {
 		decimal_t a, b, c;
 		if (! xrow_update_arg_arith_to_decimal(arg1, &a) ||
diff --git a/test/box/update.result b/test/box/update.result
index 04866006a..6ab1a5bc5 100644
--- a/test/box/update.result
+++ b/test/box/update.result
@@ -1687,7 +1687,7 @@ tests = {
     {{'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', flt_max}, {'uint64_t', uint_max}, mp_float},                     \
     {{'float', 1.0001}, {'double', 1.0000000000001}, mp_double},                \
 }
 ---
diff --git a/test/box/update.test.lua b/test/box/update.test.lua
index d52be4f3b..1538cae9c 100644
--- a/test/box/update.test.lua
+++ b/test/box/update.test.lua
@@ -624,7 +624,7 @@ tests = {
     {{'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},                    \
+    {{'float', flt_max}, {'uint64_t', uint_max}, mp_float},                     \
 -- Precision matters too. Double is used when need to avoid                     \
 -- precision loss.                                                              \
     {{'float', 1.0001}, {'double', 1.0000000000001}, mp_double},                \
-- 
2.21.1 (Apple Git-122.3)



More information about the Tarantool-patches mailing list