Re: [Tarantool-patches] [PATCH v2] tuple: drop extra restrictions for multikey index

[Nikita Pettik <korablev@tarantool.org>]

On 05 Aug 13:12, Ilya Kosarev wrote:
> @ChangeLog:
>  * Dropped restrictions on nullable multikey index root. All fields are
>  now nullable by default as it was before 2.2.1 (gh-5192).

I'd underline the fact that problem was fixed in another (local code
change) way.

What is more, I guess we should document this behaviour: our current
docs say nothing concerning nulls as multikey roots.
 
> diff --git a/test/engine/gh-5027-fields-nullability.test.lua b/test/engine/gh-5027-fields-nullability.test.lua
> index 960103d6c..664883d0d 100644
> --- a/test/engine/gh-5027-fields-nullability.test.lua
> +++ b/test/engine/gh-5027-fields-nullability.test.lua
> @@ -3,35 +3,18 @@ test_run = require('test_run').new()
>  s = box.schema.space.create('gh-5027', {engine=test_run:get_cfg('engine')})
>  _ = s:create_index('i1', {parts={{1, 'unsigned'}}})
>  _ = s:create_index('i2', {parts={{5, 'unsigned', is_nullable=true}}})
> -s:replace{1}
> -s:replace{1, box.NULL}
> -s:replace{1, box.NULL, box.NULL}
> -s:replace{1, box.NULL, box.NULL, box.NULL}
> +s:replace{1} -- ok
> +s:replace{1, box.NULL} -- ok
> +s:replace{1, box.NULL, box.NULL} -- ok
> +s:replace{1, box.NULL, box.NULL, box.NULL} -- ok

I'd drop these -- ok comments since they only enlarge diff.

> diff --git a/test/engine/gh-5192-multikey-root-nullability.test.lua b/test/engine/gh-5192-multikey-root-nullability.test.lua
> new file mode 100644
> index 000000000..5a8305bf9
> --- /dev/null
> +++ b/test/engine/gh-5192-multikey-root-nullability.test.lua
> @@ -0,0 +1,20 @@
> +test_run = require('test_run').new()
> +
> +s = box.schema.space.create('gh-5027', {engine=test_run:get_cfg('engine')})
> +_ = s:format({{name='id'}, {name='data', type='array', is_nullable=true}})
> +_ = s:create_index('i1', {parts={{1, 'unsigned'}}})
> +s:replace{1, box.NULL} -- ok
> +_ = s:create_index('i2', {parts={{field=2, path='[*].key', type='string'}}})
> +s:replace{2, box.NULL} -- ok
> +_ = s:delete(2)
> +s:truncate()

Why do you need to truncate space before drop?

> +s:drop()
> +
> +s = box.schema.space.create('gh-5027', {engine=test_run:get_cfg('engine')})
> +_ = s:format({{name='id'}, {name='data', type='array'}})
> +_ = s:create_index('i1', {parts={{1, 'unsigned'}}})
> +s:replace{1, box.NULL} -- error
> +_ = s:create_index('i2', {parts={{field=2, path='[*].key', type='string'}}})
> +s:replace{2, box.NULL} -- error
> +s:replace{3, {}} -- ok
> +s:drop()
> \ No newline at end of file

Nit: put pls newline at the end of file to avoid this warnings.

> diff --git a/test/engine/json.test.lua b/test/engine/json.test.lua
> index 4771c3162..371bbad91 100644
> --- a/test/engine/json.test.lua
> +++ b/test/engine/json.test.lua
> @@ -220,9 +220,9 @@ s:insert{4, {}} -- error
>  s:insert{5, {{b = 1}}} -- error
>  s:insert{6, {{a = 1}}} -- ok
>  s.index.sk:alter{parts = {{'[2][1].a', 'unsigned', is_nullable = true}}}
> -s:insert{7, box.NULL} -- error
> -s:insert{8, {box.NULL}} -- error
> --- Skipping nullable fields is okay though.
> +s:insert{7, box.NULL} -- ok
> +s:insert{8, {box.NULL}} -- ok
> +-- Skipping nullable fields is also okay.

Is this change really required? :)

>  s:insert{9} -- ok
>  s:insert{10, {}} -- ok
>  s:insert{11, {{b = 1}}} -- ok