Tarantool development patches archive
 help / color / mirror / Atom feed
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 4/4] tuple: use field type in update of a float/double
Date: Tue,  4 Feb 2020 23:53:47 +0100	[thread overview]
Message-ID: <fc3ac95d04fcc0c3ea98440d59567cbd3d0a88a5.1580856722.git.v.shpilevoy@tarantool.org> (raw)
In-Reply-To: <cover.1580856721.git.v.shpilevoy@tarantool.org>

There was a bug that float +/- float could result into infinity
even if result fits a double. It was fixed by storing double or
float depending on a result value. But it didn't take result field
type into account. That led to a bug when a double field +/- a
value fit the float range, and could be stored as float resulting
into an error at attempt to create a tuple.

Now if a field type is double in the tuple format, it will store
double always, even if it fits the float range.

Follow-up #4701
---
 src/box/xrow_update_field.c | 13 ++++++++++---
 test/box/update.result      | 38 +++++++++++++++++++++++++++++++++++++
 test/box/update.test.lua    | 17 +++++++++++++++++
 3 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/src/box/xrow_update_field.c b/src/box/xrow_update_field.c
index d431c22f8..cc64cf955 100644
--- a/src/box/xrow_update_field.c
+++ b/src/box/xrow_update_field.c
@@ -352,9 +352,8 @@ xrow_update_arg_arith_sizeof(const struct xrow_update_arg_arith *arg)
 		}
 		break;
 	case XUPDATE_TYPE_DOUBLE:
-		return mp_sizeof_double(arg->dbl);
 	case XUPDATE_TYPE_FLOAT:
-		return mp_sizeof_float(arg->flt);
+		return mp_sizeof_double(arg->dbl);
 	default:
 		assert(arg->type == XUPDATE_TYPE_DECIMAL);
 		return mp_sizeof_decimal(&arg->dec);
@@ -534,7 +533,6 @@ xrow_update_op_store_arith(struct xrow_update_op *op,
 			   char *out)
 {
 	(void) format_tree;
-	(void) this_node;
 	(void) in;
 	char *begin = out;
 	struct xrow_update_arg_arith *arg = &op->arg.arith;
@@ -553,6 +551,15 @@ xrow_update_op_store_arith(struct xrow_update_op *op,
 		out = mp_encode_double(out, arg->dbl);
 		break;
 	case XUPDATE_TYPE_FLOAT:
+		if (this_node != NULL) {
+			enum field_type type =
+				json_tree_entry(this_node, struct tuple_field,
+						token)->type;
+			if (type == FIELD_TYPE_DOUBLE) {
+				out = mp_encode_double(out, arg->flt);
+				break;
+			}
+		}
 		out = mp_encode_float(out, arg->flt);
 		break;
 	default:
diff --git a/test/box/update.result b/test/box/update.result
index dfbd8714f..04866006a 100644
--- a/test/box/update.result
+++ b/test/box/update.result
@@ -1712,3 +1712,41 @@ err
 ---
 - null
 ...
+-- Check that if a field has 'double' field type, it won't use
+-- float, even when the value fits float range.
+s = box.schema.create_space('test', {                                           \
+    format = {{'field1', 'unsigned'}, {'field2', 'double'}, {'field3', 'any'}}  \
+})
+---
+...
+_ = s:create_index('pk')
+---
+...
+dbl1 = ffi.cast('double', 1)
+---
+...
+_ = s:replace{1, dbl1, 1}
+---
+...
+s:update({1}, {{'+', 2, dbl1}})
+---
+- [1, 2, 1]
+...
+_ = s:delete{1}
+---
+...
+-- Check deep fields.
+_ = s:create_index('deep_sk', {'field3', 'double', path = '[1].key1.key2[2]'})
+---
+- error: Illegal parameters, unexpected option '1'
+...
+_ = s:replace{1, dbl1, {{key1 = {key2 = {1, dbl1}}}}}
+---
+...
+s:update({1}, {{'+', 'field3[1].key1.key2[2]', dbl1}})
+---
+- [1, 1, [{'key1': {'key2': [1, 2]}}]]
+...
+s:drop()
+---
+...
diff --git a/test/box/update.test.lua b/test/box/update.test.lua
index 74f9c62e2..d52be4f3b 100644
--- a/test/box/update.test.lua
+++ b/test/box/update.test.lua
@@ -642,3 +642,20 @@ for i, test in pairs(tests) do
     end                                                                         \
 end
 err
+
+-- Check that if a field has 'double' field type, it won't use
+-- float, even when the value fits float range.
+s = box.schema.create_space('test', {                                           \
+    format = {{'field1', 'unsigned'}, {'field2', 'double'}, {'field3', 'any'}}  \
+})
+_ = s:create_index('pk')
+dbl1 = ffi.cast('double', 1)
+_ = s:replace{1, dbl1, 1}
+s:update({1}, {{'+', 2, dbl1}})
+_ = s:delete{1}
+-- Check deep fields.
+_ = s:create_index('deep_sk', {'field3', 'double', path = '[1].key1.key2[2]'})
+_ = s:replace{1, dbl1, {{key1 = {key2 = {1, dbl1}}}}}
+s:update({1}, {{'+', 'field3[1].key1.key2[2]', dbl1}})
+
+s:drop()
-- 
2.21.1 (Apple Git-122.3)

  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 ` [Tarantool-patches] [PATCH v2 1/4] tuple: don't truncate float in :update() Vladislav Shpilevoy
2020-02-10 14:41   ` 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 ` Vladislav Shpilevoy [this message]
2020-02-10 16:16   ` [Tarantool-patches] [PATCH v2 4/4] tuple: use field type in update of a float/double 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=fc3ac95d04fcc0c3ea98440d59567cbd3d0a88a5.1580856722.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 4/4] tuple: use field type in update of a float/double' \
    /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