From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp58.i.mail.ru (smtp58.i.mail.ru [217.69.128.38]) (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 5D0654696C3 for ; Wed, 5 Feb 2020 01:53:53 +0300 (MSK) From: Vladislav Shpilevoy Date: Tue, 4 Feb 2020 23:53:47 +0100 Message-Id: In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH v2 4/4] tuple: use field type in update of a float/double List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org, kostja.osipov@gmail.com, imun@tarantool.org, korablev@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)