Re: [Tarantool-patches] [PATCH v1 4/4] sql: introduce decimal to arithmetic

[Safin Timur via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

On 16.08.2021 18:57, Mergen Imeev via Tarantool-patches wrote:
> This patch introduces arithmetic for DECIMAL in SQL. After this patch,
> DECIMAL values can participate in arithmetic along with INTEGER,
> UNSIGNED, DOUBLE, and other DECIMAL values.
> 
> Part of #4415
> ---
>   src/box/sql/mem.c             | 124 +++++++++++++++++++++++++++++-
>   test/sql-tap/decimal.test.lua | 141 +++++++++++++++++++++++++++++++++-
>   2 files changed, 262 insertions(+), 3 deletions(-)
> 
> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
> index ff8b40d7f..a4ec98f34 100644
> --- a/src/box/sql/mem.c
> +++ b/src/box/sql/mem.c
> @@ -1733,6 +1733,18 @@ mem_get_int(const struct Mem *mem, int64_t *i, bool
> *is_neg)
>   		}
>   		return -1;
>   	}
> +	if (mem->type == MEM_TYPE_DEC) {
> +		if (decimal_is_neg(&mem->u.d)) {
> +			if (decimal_to_int64(&mem->u.d, i) == NULL)
> +				return -1;
> +			*is_neg = *i < 0;
> +			return 0;
> +		}
> +		if (decimal_to_uint64(&mem->u.d, (uint64_t *)i) == NULL)
> +			return -1;
> +		*is_neg = false;
> +		return 0;
> +	}
>   	return -1;
>   }
>   
> @@ -1760,6 +1772,19 @@ mem_get_uint(const struct Mem *mem, uint64_t *u)
>   		}
>   		return -1;
>   	}
> +	if (mem->type == MEM_TYPE_DEC) {
> +		if (decimal_is_neg(&mem->u.d)) {
> +			int64_t i;
> +			if (decimal_to_int64(&mem->u.d, &i) == NULL || i <
> 0)
> +				return -1;
> +			assert(i == 0);
> +			*u = 0;
> +			return 0;
> +		}
> +		if (decimal_to_uint64(&mem->u.d, u) == NULL)
> +			return -1;
> +		return 0;
> +	}
>   	return -1;
>   }
>   
> @@ -1778,6 +1803,10 @@ mem_get_double(const struct Mem *mem, double *d)
>   		*d = (double)mem->u.u;
>   		return 0;
>   	}
> +	if (mem->type == MEM_TYPE_DEC) {
> +		*d = atof(decimal_str(&mem->u.d));
> +		return 0;
> +	}

1st question is - was it intentionally that you call here atof, while 
few lines belowe we use sqlAtoF?

2nd complain is - it all looks that we miss decimal_to_double() function 
in src/core/decimal.c (where we do have decimal_from_double() but not 
the reverse direction). It will look more consistent if this 
implementation would be there. And, I stll believe, there is better way 
for converting decimal to double, than converting it to string, and then 
to double.

[Though quick navigation over decNumber API didn't reveal the direct 
approach. Serge, could you please recommend the easiest way here?]


> @@ -1946,12 +2003,12 @@ mem_concat(struct Mem *a, struct Mem *b, struct
> Mem *result)
>   static inline int
>   check_types_numeric_arithmetic(const struct Mem *a, const struct Mem *b)
>   {
> -	if (!mem_is_num(a) || mem_is_metatype(a) || a->type ==
> MEM_TYPE_DEC) {
> +	if (!mem_is_num(a) || mem_is_metatype(a)) {

And now it looks better than before, and less confusing, I do not have 
complains here anymore...


>   		diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(a),
>   			 "integer, unsigned or double");
>   		return -1;
>   	}
> -	if (!mem_is_num(b) || mem_is_metatype(b) || b->type ==
> MEM_TYPE_DEC) {
> +	if (!mem_is_num(b) || mem_is_metatype(b)) {

And here.

>   		diag_set(ClientError, ER_SQL_TYPE_MISMATCH, mem_str(b),
>   			 "integer, unsigned or double");
>   		return -1;

Thanks,
Timur