[Tarantool-patches] [PATCH 4/6] sql: remove mem_apply_type() from OP_MustBeInt

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Mon Jun 1 20:04:07 MSK 2020


Thanks for the patch!

See 3 comments below.

On 28/05/2020 16:17, Mergen Imeev wrote:
> This patch replaces mem_apply_type() by mem_check_types() in
> OP_MustBeInt, which allows to remove implicit case in some places,

1. 'case' -> 'cast'.

> for example in IN operator.
> 
> Part of #4230
> ---
> diff --git a/test/sql-tap/tkt-fc7bd6358f.test.lua b/test/sql-tap/tkt-fc7bd6358f.test.lua
> index fe5d6200f..f38ffa3d6 100755
> --- a/test/sql-tap/tkt-fc7bd6358f.test.lua
> +++ b/test/sql-tap/tkt-fc7bd6358f.test.lua
> @@ -80,7 +80,6 @@ for a, from in ipairs(froms) do
>              function()
>                  return test:execsql(string.format("SELECT t1.textid, i.intid, t2.textid %s %s", from, where))
>              end, {
> -                "12", 12, "12", "34", 34, "34"
>              })
>  
>          test:do_test(
> @@ -88,7 +87,6 @@ for a, from in ipairs(froms) do
>              function()
>                  return test:execsql(string.format("SELECT t1.textid, i.intid, t2.textid %s %s", from, where))
>              end, {
> -                "12", 12, "12", "34", 34, "34"
>              })

2. In the header of this file it is said, that the whole test's
purpose is to ensure, that these values are returned. Do we
need this test file now at all? Or can it be fixed in a way, that
the results are not changed?

>  
>      end
> diff --git a/test/sql-tap/whereB.test.lua b/test/sql-tap/whereB.test.lua
> index fe5e28c70..970ff1dec 100755
> --- a/test/sql-tap/whereB.test.lua
> +++ b/test/sql-tap/whereB.test.lua
> @@ -432,7 +432,6 @@ test:do_execsql_test(
>      ]],
>      {
>      -- <whereB-5.2>
> -    1, 2, true
>      -- </whereB-5.2>
>      })
>  
> @@ -443,7 +442,6 @@ test:do_execsql_test(
>      ]],
>      {
>      -- <whereB-5.3>
> -    1, 2, true

3. These tests also look useless now. Their comment
says:

	-- Because t2.b has a numeric affinity, type conversion should occur
	-- and the two fields should be equal.

And now they are not equal. Do we need these tests?


More information about the Tarantool-patches mailing list