Re: [Tarantool-patches] [PATCH v1 2/2] sql: disallow explicit cast of VARBINARY to number

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

Thanks for the patch!

See 3 comments below.

On 21.07.2021 17:10, Mergen Imeev via Tarantool-patches wrote:
> This patch removes explicit cast of VARBINARY values to numeric types.
> 
> Part of #4470
> Closes #4772
> Closes #5852

1. You might need to add a changelog file.

> diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
> index 3dc49c38e..379fbf09e 100755
> --- a/test/sql-tap/cast.test.lua
> +++ b/test/sql-tap/cast.test.lua
> @@ -867,13 +879,14 @@ test:do_test(
>  -- gh-4356: Check that result of blob to number cast if of type
>  -- number.

2. This comment could mean the test becomes just wrong and can
be deleted.

> diff --git a/test/sql-tap/numcast.test.lua b/test/sql-tap/numcast.test.lua
> index 56b11da25..a2877167f 100755
> --- a/test/sql-tap/numcast.test.lua
> +++ b/test/sql-tap/numcast.test.lua
> @@ -149,20 +149,22 @@ test:do_execsql_test(
>  -- gh-4233: Make sure that NUMBER can contain UNSIGNED, INTEGER
>  -- and DOUBLE and is not automatically converted to DOUBLE.
>  --

3. These tests seem to be quite far from this comment. Perhaps they
are broken for some time already. Might worth deleting them.

> -test:do_execsql_test(
> +test:do_catchsql_test(
>      "numcast-3.1",
>      [[
>          SELECT CAST(x'3131313131313131313131313131313131313131' AS NUMBER);
>      ]], {
> -        11111111111111111111ULL
> +        1, "Type mismatch: can not convert "..
> +           "varbinary(x'3131313131313131313131313131313131313131') to number"
>      })
>  
> -test:do_execsql_test(
> +test:do_catchsql_test(
>      "numcast-3.2",
>      [[
>          SELECT CAST(x'31313131313131313131313131313131313131312E' AS NUMBER);
>      ]], {
> -        11111111111111110656
> +        1, "Type mismatch: can not convert "..
> +           "varbinary(x'31313131313131313131313131313131313131312E') to number"
>      })
>  
>  test:do_execsql_test(
>