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

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

[Ilya Kosarev]

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()
-- 
2.17.1

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

[Aleksandr Lyapunov]

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()

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

[Kirill Yukhin]

Hello,

On 08 июл 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

I've checked your patch into 2.3, 2.4 and master.

--
Regards, Kirill Yukhin
> 
> @ChangeLog:
>  * Fix assert outdated due to multikey index arrival.

In future, please mention issue in brackets: (gh-5132).

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

[Ilya Kosarev]

[-- Attachment #1: Type: text/plain, Size: 966 bytes --]


Hi!
  
>Понедельник, 13 июля 2020, 14:04 +03:00 от Kirill Yukhin <kyukhin@tarantool.org>:
> 
>Hello,
>
>On 08 июл 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
>
>I've checked your patch into 2.3, 2.4 and master.
>
>--
>Regards, Kirill Yukhin
>>
>> @ChangeLog:
>> * Fix assert outdated due to multikey index arrival.
>
>In future, please mention issue in brackets: (gh-5132).
Right, sorry.
 
--
Ilya Kosarev
 

[-- Attachment #2: Type: text/html, Size: 1704 bytes --]