Re: [Tarantool-patches] [PATCH v1 1/1] sql: allow to convert big real values to integer

[Nikita Pettik <korablev@tarantool.org>]

On 05 Nov 20:42, imeevma@tarantool.org wrote:
> diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
> index 2d37b62..82daac7 100644
> --- a/src/box/sql/vdbemem.c
> +++ b/src/box/sql/vdbemem.c
> @@ -564,10 +564,16 @@ sqlVdbeMemIntegerify(Mem * pMem, bool is_forced)
>  		mem_set_int(pMem, i, is_neg);
>  		return 0;
>  	} else if ((pMem->flags & MEM_Real) != 0 && is_forced) {
> -		if (pMem->u.r >= INT64_MAX || pMem->u.r < INT64_MIN)
> -			return -1;
> -		mem_set_int(pMem, pMem->u.r, pMem->u.r <= -1);
> -		return 0;
> +		if (pMem->u.r < INT64_MAX && pMem->u.r >= INT64_MIN) {
> +			mem_set_int(pMem, pMem->u.r, pMem->u.r <= -1);
> +			return 0;
> +		}
> +		if (pMem->u.r >= INT64_MAX && pMem->u.r < UINT64_MAX) {
> +			uint64_t val = pMem->u.r;
> +			mem_set_int(pMem, (int64_t)val, false);

Use mem_set_u64() instead. BTW below I see path which is executed when
is_forced == false. And it is also about same wrong conversion. Please
check it as well and add test case.

> +			return 0;
> +		}
> +		return -1;
>  	}
>  
>  	double d;
> diff --git a/test/sql/integer-overflow.test.lua b/test/sql/integer-overflow.test.lua
> index 1b3e8ce..5260639 100644
> --- a/test/sql/integer-overflow.test.lua
> +++ b/test/sql/integer-overflow.test.lua
> @@ -32,7 +32,7 @@ box.execute('SELECT CAST(\'18446744073709551616\' AS INTEGER);')
>  -- float 9223372036854775800 -> int (9223372036854775808),
>  -- with error due to conversion = 8.
>  --
> -box.execute('SELECT CAST(9223372036854775807.0 AS INTEGER);')
> +box.execute('SELECT CAST(18446744073709551616. AS INTEGER);')

Why did you changed this test?

>  -- gh-3810: make sure that if space contains integers in range
>  -- [INT64_MAX, UINT64_MAX], they are handled inside SQL in a
>  -- proper way, which now means that an error is raised.
> -- 
> 2.7.4
>