From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <korablev@tarantool.org>
Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251])
 (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 7ED9646970E
 for <tarantool-patches@dev.tarantool.org>;
 Mon, 10 Feb 2020 17:41:16 +0300 (MSK)
Date: Mon, 10 Feb 2020 17:41:15 +0300
From: Nikita Pettik <korablev@tarantool.org>
Message-ID: <20200210144115.GH1110@tarantool.org>
References: <cover.1580856721.git.v.shpilevoy@tarantool.org>
 <1a28ebaa58b81f4cbc3a1656cf364f281919ade9.1580856721.git.v.shpilevoy@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <1a28ebaa58b81f4cbc3a1656cf364f281919ade9.1580856721.git.v.shpilevoy@tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH v2 1/4] tuple: don't truncate float
	in :update()
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org

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 <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;
>  		}

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.

> -		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;