Re: [Tarantool-patches] [PATCH v5 19/52] sql: introduce arithmetic operations for MEM

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Good job on the fixes!

>> The names could be mem_arith_plus(), mem_arith_mul(), mem_arith_minus(),
>> etc.
> Fixed. I named new functions mem_add(), mem_sub(), mem_mul(), mem_div() and
> mem_rem(). Each of them simpler than this function.

The last operation is called modulo. Usually shortened to mod. But I see
we already use 'rem' in some other place, and in the token name. Up to you.

See 1 comment below.

> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
> index 2d76ef88d..859e337aa 100644
> --- a/src/box/sql/mem.c
> +++ b/src/box/sql/mem.c
> @@ -390,6 +390,240 @@ mem_concat(struct Mem *a, struct Mem *b, struct Mem *result)
> +
> +static int
> +get_number(const struct Mem *mem, struct sql_num *number)
> +{
> + if ((mem->flags & MEM_Real) != 0) {
> +   number->d = mem->u.r;
> +   number->type = MEM_Real;
> +   return 0;
> + }
> + if ((mem->flags & MEM_Int) != 0) {
> +   number->i = mem->u.i;
> +   number->type = MEM_Int;
> +   number->is_neg = true;
> +   return 0;
> + }
> + if ((mem->flags & MEM_UInt) != 0) {
> +   number->u = mem->u.u;
> +   number->type = MEM_UInt;
> +   number->is_neg = false;
> +   return 0;
> + }
> + if ((mem->flags & (MEM_Str | MEM_Blob)) == 0)
> +   return -1;
> + if ((mem->flags & MEM_Subtype) != 0)
> +   return -1;
> + if (sql_atoi64(mem->z, &number->i, &number->is_neg, mem->n) == 0) {
> +   number->type = number->is_neg ? MEM_Int : MEM_UInt;
> +   /*
> +    * The next line should be removed along with the is_neg field
> +    * of struct sql_num. The integer type tells us about the sign.
> +    * However, if it is removed, the behavior of arithmetic
> +    * operations will change.
> +    */
> +   number->is_neg = (mem->flags & MEM_Int) != 0;

I don't understand that. How is it possible it mismatches the
value returned from sql_atoi64()? And why isn't it just 'false' then?
Because a few lines above you already checked (mem->flags & MEM_Int) != 0
and it was false.

> +   return 0;
> + }
> + if (sqlAtoF(mem->z, &number->d, mem->n) != 0) {
> +   number->type = MEM_Real;
> +   return 0;
> + }
> + return -1;
> +}