Re: [Tarantool-patches] [PATCH v4 1/3] sql: fix CAST() from STRING to INTEGER

[Nikita Pettik <korablev@tarantool.org>]

On 27 Mar 14:33, imeevma@tarantool.org wrote:

Could you please find Peter's table containing current/expected cast
behaviours and verify that this patch doesn't contradict it?

> diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
> index aad030d..de1d9c3 100644
> --- a/src/box/sql/vdbemem.c
> +++ b/src/box/sql/vdbemem.c
> @@ -696,7 +696,7 @@ sqlVdbeMemCast(Mem * pMem, enum field_type type)
>  		return -1;
>  	case FIELD_TYPE_INTEGER:
>  	case FIELD_TYPE_UNSIGNED:
> -		if ((pMem->flags & MEM_Blob) != 0) {
> +		if ((pMem->flags & (MEM_Blob | MEM_Str)) != 0) {
>  			bool is_neg;
>  			int64_t val;
>  			if (sql_atoi64(pMem->z, &val, &is_neg, pMem->n) != 0)
> @@ -711,8 +711,20 @@ sqlVdbeMemCast(Mem * pMem, enum field_type type)
>  			MemSetTypeFlag(pMem, MEM_UInt);
>  			return 0;
>  		}
> -		if (sqlVdbeMemIntegerify(pMem) != 0)
> +		if ((pMem->flags & MEM_Real) != 0) {
> +			double d;
> +			if (sqlVdbeRealValue(pMem, &d) != 0)
> +				return -1;
> +			if (d < INT64_MAX && d >= INT64_MIN) {
> +				mem_set_int(pMem, d, d <= -1);
> +				return 0;
> +			}
> +			if (d >= INT64_MAX && d < UINT64_MAX) {
> +				mem_set_u64(pMem, d);
> +				return 0;
> +			}
>  			return -1;

Instead of keeping inlining code into sqlVdbeMemCast() I'd better
split it into separate functions. Good refactoring task tho - just
keep in mind.

> +		}
>  		if (type == FIELD_TYPE_UNSIGNED &&
>  		    (pMem->flags & MEM_UInt) == 0)
>  			return -1;
> diff --git a/test/sql-tap/gh-4766-wrong-cast-from-blob-to-int.test.lua b/test/sql-tap/gh-4766-wrong-cast-from-blob-to-int.test.lua
> new file mode 100755
> index 0000000..0865c4e
> --- /dev/null
> +++ b/test/sql-tap/gh-4766-wrong-cast-from-blob-to-int.test.lua

Strictly speaking this change is not related to 4766, so I'd not
create separate test file for current patch. Moreover, tests in
sql/types.test.lua seem to cover it.

> +#!/usr/bin/env tarantool
> +test = require("sqltester")
> +test:plan(6)
> +
> +--
> +-- Make sure that STRING or BLOB that contains DOUBLE value cannot
> +-- be cast to INTEGER.
> +--
> +test:do_catchsql_test(
> +    "gh-4766-1",
> +    [[
> +        SELECT CAST('1.1' AS INTEGER)
> +    ]], {
> +        1, "Type mismatch: can not convert 1.1 to integer"
> +    })
> +
> +test:do_catchsql_test(
> +    "gh-4766-2",
> +    [[
> +        SELECT CAST(x'312e31' AS INTEGER)
> +    ]], {
> +        1, "Type mismatch: can not convert varbinary to integer"
> +    })
> +
> diff --git a/test/sql/types.result b/test/sql/types.result
> index 38e4385..54aff46 100644
> --- a/test/sql/types.result
> +++ b/test/sql/types.result
> @@ -269,11 +269,8 @@ box.space.T1:drop()
>  --
>  box.execute("SELECT CAST('1.123' AS INTEGER);")
>  ---
> -- metadata:
> -  - name: CAST('1.123' AS INTEGER)
> -    type: integer
> -  rows:
> -  - [1]
> +- null
> +- 'Type mismatch: can not convert 1.123 to integer'
>  ...
>  box.execute("CREATE TABLE t1 (f TEXT PRIMARY KEY);")
>  ---
> @@ -285,13 +282,8 @@ box.execute("INSERT INTO t1 VALUES('0.0'), ('1.5'), ('3.9312453');")
>  ...
>  box.execute("SELECT CAST(f AS INTEGER) FROM t1;")
>  ---
> -- metadata:
> -  - name: CAST(f AS INTEGER)
> -    type: integer
> -  rows:
> -  - [0]
> -  - [1]
> -  - [3]
> +- null
> +- 'Type mismatch: can not convert 0.0 to integer'
>  ...
>  box.space.T1:drop()
>  ---
> @@ -1105,8 +1097,11 @@ box.execute("SELECT CAST(1.5 AS UNSIGNED);")
>  ...
>  box.execute("SELECT CAST(-1.5 AS UNSIGNED);")
>  ---
> -- null
> -- 'Type mismatch: can not convert -1 to unsigned'
> +- metadata:
> +  - name: CAST(-1.5 AS UNSIGNED)
> +    type: unsigned
> +  rows:
> +  - [-1]
>  ...
>  box.execute("SELECT CAST(true AS UNSIGNED);")
>  ---
> -- 
> 2.7.4
>