Re: [Tarantool-patches] [PATCH v1 1/1] sql: introduce literals for DECIMAL

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

Hi! Thanks for the fixes!

LGTM.

>>> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
>>> index 44533fb3e..747fb608b 100644
>>> --- a/src/box/sql/vdbe.c
>>> +++ b/src/box/sql/vdbe.c
>>> @@ -804,6 +804,17 @@ case OP_Real: {            /* same as TK_FLOAT, out2 */
>>>  	break;
>>>  }
>>>  
>>> +/* Opcode: Decimal * P2 * P4 *
>>> + * Synopsis: r[P2]=P4
>>> + *
>>> + * P4 is a pointer to a DECIMAL value. Write that value into register P2.
>>> + */
>>> +case OP_Decimal: {            /* same as TK_DECIMAL, out2 */
>>
>> 2. Lets stick to our code style when write comments:
>> start them with /** out of functions, and not write them
>> on the same line as code.
>>
> Fixed.

Nope, it seems not.

>>> diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
>>> index 68d2ae585..5f8e885ce 100755
>>> --- a/test/sql-tap/cast.test.lua
>>> +++ b/test/sql-tap/cast.test.lua
>>> @@ -962,7 +962,7 @@ test:do_catchsql_test(
>>>  test:do_catchsql_test(
>>>      "cast-6.2.3",
>>>      [[
>>> -        SELECT CAST(1.5 AS BOOLEAN);
>>> +        SELECT CAST(15e-1 AS BOOLEAN);
>>
>> 5. Please, add some tests for so large numbers that only
>> DECIMAL would fit them. To prove that VDBE really uses
>> decimal for them.
>>
> Not sure if I got it right. At first we wanted to parse large numbers without
> decimal point and exponent as DECIMAL if they are not fit for INTEGER, but we
> decided to abandon this idea. Is that what you mean?

I mean just use large decimal numbers, which wouldn't fit into an INTEGER.
You define decimals as "A value consisting of digits and a decimal point".
My proposal was to test a huge number with a decimal point which would
be > UINT64_MAX. I see you got it correct in the new patch.