[tarantool-patches] Re: [PATCH 6/6] sql: allow to specify UNSIGNED column type

[n.pettik]

>> diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
>> index 325c54c18..b6b5cd0bf 100644
>> --- a/src/box/sql/vdbeaux.c
>> +++ b/src/box/sql/vdbeaux.c
>> @@ -2887,43 +2887,50 @@ sqlBlobCompare(const Mem * pB1, const Mem * pB2)
>>        return n1 - n2;
>> }
>> 
>> -/*
>> - * Do a comparison between a 64-bit signed integer and a 64-bit floating-point
>> - * number.  Return negative, zero, or positive if the first (i64) is less than,
>> - * equal to, or greater than the second (double).
>> +/**
>> + * Do a comparison between a 64-bit unsigned/signed integer and a
>> + * 64-bit floating-point number.  Return negative, zero, or
>> + * positive if the first (integer) is less than, equal to, or
>> + * greater than the second (double).
>>  */
>> static int
>> -sqlIntFloatCompare(i64 i, double r)
>> +compare_uint_float(uint64_t u, double r)
> 
> Unfortunately, it is not as simple as you implemented it.
> See mp_compare_double_uint64 in tuple_compare.cc for details.
> In short, your function works wrong when numbers are near
> uint maximum. Perhaps, it is worth moving this comparator
> from tuple_compare.cc to a header. Like trivia/util.h.

Hi,

I remember your concern about compare_uint_float().
At the moment of review I said that integrating
mp_compare_double_uint64() into SQL code results in
test fails. In fact, that’s not true: I accidentally added
bug when substituting them. Now I've carefully reviewed
patch and it seems that all tests except for one (which
exactly covers that border case) are passed. What is more,
fix seems to be quite trivial:

@@ -2895,7 +2897,7 @@ sqlBlobCompare(const Mem * pB1, const Mem * pB2)
 static int
 compare_uint_float(uint64_t u, double r)
 {
-       if (r > (double) UINT64_MAX)
+       if (r >= exp2(64))
                return -1;
        if (r < 0.0)
                return +1;

With this fix I’ve failed to come up with counter-example
demonstrating different from  mp_compare_double_uint64
results. Anyway, I replaced compare_uint_float/compare_int_float
with analogue from comparators so that we have unified way of 
int-float comparison. Patch is on its way.