From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <vdavydov.dev@gmail.com>
Date: Thu, 13 Jun 2019 19:07:49 +0300
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Subject: Re: [PATCH v4 2/2] lib/core: introduce decimal type to tarantool
Message-ID: <20190613160749.247c3tqlftblekpi@esperanza>
References: <cover.1560268286.git.sergepetrenko@tarantool.org>
 <44257abefbafcb742f6757c4befa271517dfb4f4.1560268286.git.sergepetrenko@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <44257abefbafcb742f6757c4befa271517dfb4f4.1560268286.git.sergepetrenko@tarantool.org>
To: Serge Petrenko <sergepetrenko@tarantool.org>
Cc: georgy@tarantool.org, kostja@tarantool.org, tarantool-patches@freelists.org
List-ID: <tarantool-patches.dev.tarantool.org>

On Tue, Jun 11, 2019 at 06:56:48PM +0300, Serge Petrenko wrote:
> Add fixed-point decimal type to tarantool core.
> Adapt decNumber floating-point decimal library for the purpose, write a
> small wrapper and add unit tests.
> 
> A new decimal type is an alias for decNumber numbers from the decNumber
> library.
> Arithmetic operations (+, -, *, /) and some mathematic functions
> (ln, log10, exp, pow, sqrt) are available together with methods to
> pack and unpack decimal to and from its packed representation (useful
> for serialization).
> 
> We introduce a single context for all the arithmetic operations
> on decimals, which enforces both number precision and scale to be
> in range [0, 38]. NaNs and Infinities are restricted.
> 
> Part of #692
> ---
>  CMakeLists.txt              |   7 +
>  cmake/BuildDecNumber.cmake  |  14 ++
>  src/CMakeLists.txt          |   1 +
>  src/lib/core/CMakeLists.txt |   3 +-
>  src/lib/core/decimal.c      | 354 +++++++++++++++++++++++++++++++
>  src/lib/core/decimal.h      | 206 ++++++++++++++++++
>  test/unit/CMakeLists.txt    |   2 +
>  test/unit/decimal.c         | 174 ++++++++++++++++
>  test/unit/decimal.result    | 406 ++++++++++++++++++++++++++++++++++++
>  9 files changed, 1166 insertions(+), 1 deletion(-)
>  create mode 100644 cmake/BuildDecNumber.cmake
>  create mode 100644 src/lib/core/decimal.c
>  create mode 100644 src/lib/core/decimal.h
>  create mode 100644 test/unit/decimal.c
>  create mode 100644 test/unit/decimal.result

I squashed this patch with the previous one, because they don't make
sense without each other, and pushed to master with a few very minor
nitpicks (see below).

> +decimal_t *
> +decimal_from_double(decimal_t *dec, double d)
> +{
> +	char buf[DECIMAL_MAX_DIGITS+3];
> +	if (isinf(d) || isnan(d))
> +		return NULL;
> +	snprintf(buf, DECIMAL_MAX_DIGITS+3, "%.*f", DBL_DIG, d);

Replaced with sizeof(buf).

> +/**
> + * Initialize a decimal with an integer value.
> + *
> +*/

Removed extra line in the comment.

> +/**
> + * Using a packed representation of size \a len pointed to by
> + * *data, unpack it to \a dec.
> + *
> + * \post *data = *data + decimal_len(dec);
> + *
> + * @return NULL if value encoding is incorrect
> + *         dec otherwise.
> + */
> +decimal_t *
> +decimal_unpack(const char **data, decimal_t *dec, uint32_t len);

Swapped 'len' and 'dec' as we typically place out arguments at the end.