Re: [Tarantool-patches] [PATCH] engine: fix assert for multikey indexes

[Aleksandr Lyapunov <alyapunov@tarantool.org>]

Hi, thanks for the patch! LGTM

On 08.07.2020 00:19, Ilya Kosarev wrote:
> Since 4273ec52e122d6d37c8deedf1bc10732a7e40c0e (box: introduce JSON
> Indexes) we can create multikey index using array which might be the
> first tuple field. It technically breaks assertion which implies that
> first tuple field can't have offset in the tuple field map. Now the
> assert us updated correspondingly. According test case is added.
>
> Closes #5132
> ---
> Branch: https://github.com/tarantool/tarantool/tree/i.kosarev/gh-5132-multiey-index-wrong-format-assert
> Issue: https://github.com/tarantool/tarantool/issues/5132
>
> @ChangeLog:
>   * Fix assert outdated due to multikey index arrival.
>
>   src/box/tuple_format.c                        |  4 ++--
>   .../gh-5132-multikey-index-assert.result      | 22 +++++++++++++++++++
>   .../gh-5132-multikey-index-assert.test.lua    |  9 ++++++++
>   3 files changed, 33 insertions(+), 2 deletions(-)
>   create mode 100644 test/engine/gh-5132-multikey-index-assert.result
>   create mode 100644 test/engine/gh-5132-multikey-index-assert.test.lua
>
> diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
> index 68ec2a749..e9920480a 100644
> --- a/src/box/tuple_format.c
> +++ b/src/box/tuple_format.c
> @@ -478,8 +478,8 @@ tuple_format_create(struct tuple_format *format, struct key_def * const *keys,
>   		}
>   	}
>   
> -	assert(tuple_format_field(format, 0)->offset_slot ==
> -	       TUPLE_OFFSET_SLOT_NIL);
> +	assert(tuple_format_field(format, 0)->offset_slot == TUPLE_OFFSET_SLOT_NIL
> +	       || json_token_is_multikey(&tuple_format_field(format, 0)->token));
>   	size_t field_map_size = -current_slot * sizeof(uint32_t);
>   	if (field_map_size > UINT16_MAX) {
>   		/** tuple->data_offset is 16 bits */
> diff --git a/test/engine/gh-5132-multikey-index-assert.result b/test/engine/gh-5132-multikey-index-assert.result
> new file mode 100644
> index 000000000..dbd2e1b5f
> --- /dev/null
> +++ b/test/engine/gh-5132-multikey-index-assert.result
> @@ -0,0 +1,22 @@
> +-- test-run result file version 2
> +test_run = require('test_run').new()
> + | ---
> + | ...
> +
> +space = box.schema.space.create('gh-5132-multikey', {engine = test_run:get_cfg('engine')})
> + | ---
> + | ...
> +space:format({{name = "attributes", type = "array"}, {name = "uid", type = "string"}})
> + | ---
> + | ...
> +_ = space:create_index('primary', {name = "uid", parts = {{field = "uid"}}})
> + | ---
> + | ...
> +
> +_ = space:create_index('secondary', {name = "kv", parts = {{field = "attributes", path = "[*].key", type = "string"}}})
> + | ---
> + | ...
> +
> +space:drop()
> + | ---
> + | ...
> diff --git a/test/engine/gh-5132-multikey-index-assert.test.lua b/test/engine/gh-5132-multikey-index-assert.test.lua
> new file mode 100644
> index 000000000..3394456de
> --- /dev/null
> +++ b/test/engine/gh-5132-multikey-index-assert.test.lua
> @@ -0,0 +1,9 @@
> +test_run = require('test_run').new()
> +
> +space = box.schema.space.create('gh-5132-multikey', {engine = test_run:get_cfg('engine')})
> +space:format({{name = "attributes", type = "array"}, {name = "uid", type = "string"}})
> +_ = space:create_index('primary', {name = "uid", parts = {{field = "uid"}}})
> +
> +_ = space:create_index('secondary', {name = "kv", parts = {{field = "attributes", path = "[*].key", type = "string"}}})
> +
> +space:drop()