From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp45.i.mail.ru (smtp45.i.mail.ru [94.100.177.105]) (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 61CF146970E for ; Tue, 11 Feb 2020 00:18:48 +0300 (MSK) References: <1a28ebaa58b81f4cbc3a1656cf364f281919ade9.1580856721.git.v.shpilevoy@tarantool.org> <20200210144115.GH1110@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Mon, 10 Feb 2020 22:18:46 +0100 MIME-Version: 1.0 In-Reply-To: <20200210144115.GH1110@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v2 1/4] tuple: don't truncate float in :update() List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the review! On 10/02/2020 15:41, Nikita Pettik wrote: > On 04 Feb 23:53, Vladislav Shpilevoy wrote: >> 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 >> + >> /* {{{ 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; >> } > > Patch is ok, but I'd add a brief comment here like: > > If value fits into float, then store it as a float; > store it as double otherwise. > > Up to you. Added a comment: ================================================================================ diff --git a/src/box/xrow_update_field.c b/src/box/xrow_update_field.c index 6ac29de5d..0540e8c3e 100644 --- a/src/box/xrow_update_field.c +++ b/src/box/xrow_update_field.c @@ -403,6 +403,12 @@ xrow_update_arith_make(struct xrow_update_op *op, break; } float fc = (float) c; + /* + * If fits into float - store as a float. + * Otherwise as a double. Fits means, that the + * value precision is not truncated by float, and + * its value is inside the float range. + */ if ((lowest_type == XUPDATE_TYPE_DOUBLE && c != (double) fc) || (lowest_type == XUPDATE_TYPE_FLOAT && (c >= FLT_MAX || c <= -FLT_MAX))) {