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

[Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

16.08.2021 22:48, Safin Timur пишет:
>
>
> 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?]

I agree, it would be best to introduce decimal_to_double() to 
src/lib/core/decimal

I can't think of an easy way here. The only one I see is construct the 
double value
digit by digit. This mustn't be too different from 
decimal->string->double conversion.

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

-- 
Serge Petrenko