Re: [Tarantool-patches] [PATCH v5 6/6] sql: remove implicit cast from MakeRecord opcode

[Nikita Pettik <korablev@tarantool.org>]

On 21 Aug 12:19, imeevma@tarantool.org wrote:
> This patch removes implicit casting from MakeRecord opcode.
> 
> Closes #4230
> 
> @TarantoolBot document
> Title: remove implicit cast for comparison
> 
> After this patch-set, there will be no implicit casts for comparison.
> This means that the values ​of the field types STRING, BOOLEAN and

Again these strange characters in commit msg..

> VARBINARY can be compared with the values of the same field type.

Only.

> Any numerical value can be compared with any other numerical value.
> 
> Example:
> 
> ```
> tarantool> box.execute([[SELECT '1' > 0;]])
> ---
> - null
> - 'Type mismatch: can not convert 1 to numeric'
> ...
> 
> tarantool> box.execute([[SELECT true > X'33';]])
> ---
> - null
> - 'Type mismatch: can not convert boolean to varbinary'
> ...
> 
> tarantool> box.execute([[SELECT 1.23 > 123;]])
> ---
> - metadata:
>   - name: 1.23 > 123
>     type: boolean
>   rows:
>   - [false]
> ...
> ```
> ---
> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
> index bc2182446..d553b80db 100644
> --- a/src/box/sql/expr.c
> +++ b/src/box/sql/expr.c
> @@ -2886,11 +2886,18 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
>  						jmpIfDynamic = -1;
>  					}
>  					r3 = sqlExprCodeTarget(pParse, pE2, r1);
> -					enum field_type types[2] =
> -						{ lhs_type, field_type_MAX };
> -	 				sqlVdbeAddOp4(v, OP_MakeRecord, r3,
> -							  1, r2, (char *)types,
> -							  sizeof(types));
> +					uint32_t size =
> +						2 * sizeof(enum field_type);
> +					enum field_type *types=
> +						sqlDbMallocZero(pParse->db,

Why do you have manually alloc types? It is done in sqlVdbeAddOp4()
if pass n > 0 (like it was before your refactoring).

> +								size);
> +					types[0] = lhs_type;
> +					types[1] = field_type_MAX;
> +					sqlVdbeAddOp4(v, OP_ApplyType, r3, 1, 0,
> +						      (char *)types, P4_DYNAMIC);
> +					sqlVdbeChangeP5(v, OPFLAG_DO_NOT_CONVERT_NUMBERS);
> +					sqlVdbeAddOp3(v, OP_MakeRecord, r3, 1,
> +						      r2);
>  					sql_expr_type_cache_change(pParse,
>  								   r3, 1);
>  					sqlVdbeAddOp2(v, OP_IdxInsert, r2,

You also can remove now mem_apply_type().

> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> index a0ddbaf60..544619b03 100644
> --- a/src/box/sql/vdbe.c
> +++ b/src/box/sql/vdbe.c
> @@ -3145,24 +3145,17 @@ type_mismatch:
>  	break;
>  }
>  
> diff --git a/test/sql-tap/in3.test.lua b/test/sql-tap/in3.test.lua
> index a6d842962..7f3abbae0 100755
> --- a/test/sql-tap/in3.test.lua
> +++ b/test/sql-tap/in3.test.lua
> @@ -1,6 +1,6 @@
>  #!/usr/bin/env tarantool
>  test = require("sqltester")
> -test:plan(28)
> +test:plan(26)
>  
>  --!./tcltestrunner.lua
>  -- 2007 November 29
> @@ -334,18 +334,6 @@ test:do_test(
>          -- </in3-3.4>
>      })
>  
> -test:do_test(
> -    "in3-3.5",
> -    function()
> -        -- Numeric affinity should be applied to each side before the comparison
> -        -- takes place. Therefore we cannot use index t1_i1, which has no affinity.
> -        return exec_neph(" SELECT y IN (SELECT a FROM t1) FROM t2 ")
> -    end, {
> -        -- <in3-3.5>
> -        1, true
> -        -- </in3-3.5>
> -    })
> -
>  test:do_test(
>      "in3-3.6",
>      function()
> @@ -358,18 +346,6 @@ test:do_test(
>          -- </in3-3.6>
>      })
>  
> -test:do_test(
> -    "in3-3.7",
> -    function()
> -        -- Numeric affinity is applied before the comparison takes place. 
> -        -- Making it impossible to use index t1_i3.
> -        return exec_neph(" SELECT y IN (SELECT c FROM t1) FROM t2 ")
> -    end, {
> -        -- <in3-3.7>
> -        1, true
> -        -- </in3-3.7>
> -    })
> -

Why did you remove these tests?

>  -----------------------------------------------------------------------
>  --
>  -- Test using a multi-column index.