From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <kshcherbatov@tarantool.org>
Subject: Re: [tarantool-patches] Re: [PATCH v5 4/4] box: introduce multikey
 indexes in memtx
References: <cover.1557142159.git.kshcherbatov@tarantool.org>
 <bb1e8c779aae2538c797d45668f190ba46ce438e.1557142159.git.kshcherbatov@tarantool.org>
 <20190506154603.txsxk2hsyshdqtdb@esperanza>
 <88b86a7c-686d-30ac-fe75-f892381e96a4@tarantool.org>
 <20190507081103.fnzqguaeev4hxrlt@esperanza>
From: Kirill Shcherbatov <kshcherbatov@tarantool.org>
Message-ID: <1bedf880-3e47-4662-258c-dbc9326e0b44@tarantool.org>
Date: Tue, 7 May 2019 11:28:46 +0300
MIME-Version: 1.0
In-Reply-To: <20190507081103.fnzqguaeev4hxrlt@esperanza>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
To: tarantool-patches@freelists.org, Vladimir Davydov <vdavydov.dev@gmail.com>
List-ID: <tarantool-patches.dev.tarantool.org>


>> +	assert(field_map_get_size((uint32_t *)raw,
>> +				  format->field_map_size) == field_map_size);
> 
> Could you please move this assertion to field_map_build so that it works
> for all engines, not only for memtx?

Partially this new condition is less reliable: we use the same components
that were used a bit before on build. Now it is a check that _build action
and _get_size are compatible; I don't mind.

diff --git a/src/box/field_map.c b/src/box/field_map.c
index a917d4a25..fd696c198 100644
--- a/src/box/field_map.c
+++ b/src/box/field_map.c
@@ -114,6 +114,9 @@ field_map_build(struct field_map_builder *builder, char *buffer)
 		extent_wptr += sizeof(uint32_t) + extent_offset_sz;
 	}
 	assert(extent_wptr == buffer + builder->extents_size);
+	assert(field_map_get_size(field_map,
+				  builder->slot_count * sizeof(uint32_t)) ==
+	       field_map_build_size(builder));
 }
 
 uint32_t
diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c
index ab4913035..806a79c5a 100644
--- a/src/box/memtx_engine.c
+++ b/src/box/memtx_engine.c
@@ -1180,8 +1180,6 @@ memtx_tuple_new(struct tuple_format *format, const char *data, const char *end)
 	char *raw = (char *) tuple + tuple->data_offset;
 	field_map_build(&builder, raw - field_map_size);
 	memcpy(raw, data, tuple_len);
-	assert(field_map_get_size((uint32_t *)raw,
-				  format->field_map_size) == field_map_size);
 	say_debug("%s(%zu) = %p", __func__, tuple_len, memtx_tuple);
 end:
 	region_truncate(region, region_svp);


> 
> Also, we need to make sure that the key_def module doesn't allow to
> create multikey key definitions, as its API doesn't allow to compare
> multikey indexes. Please do it in the scope of this patch as well.
> May be, we will extend the API later to allow that, but I think we
> can live without it for now.

diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c
index 0e1236093..b4bd64f59 100644
--- a/src/box/lua/key_def.c
+++ b/src/box/lua/key_def.c
@@ -179,6 +179,11 @@ luaT_key_def_set_part(struct lua_State *L, struct key_part_def *part,
 			diag_set(IllegalParams, "invalid path");
 			return -1;
 		}
+		if ((size_t)json_path_multikey_offset(path, path_len,
+					      TUPLE_INDEX_BASE) != path_len) {
+			diag_set(IllegalParams, "multikey path is unsupported");
+			return -1;
+		}
 		char *tmp = region_alloc(region, path_len + 1);
 		if (tmp == NULL) {
 			diag_set(OutOfMemory, path_len + 1, "region", "path");

diff --git a/test/box-tap/key_def.test.lua b/test/box-tap/key_def.test.lua
index c52ff48fe..4b468a696 100755
--- a/test/box-tap/key_def.test.lua
+++ b/test/box-tap/key_def.test.lua
@@ -128,6 +128,15 @@ local key_def_new_cases = {
         }},
         exp_err = 'invalid path',
     },
+    {
+        'Multikey JSON path',
+        parts = {{
+            fieldno = 1,
+            type = 'string',
+            path = '[*]',
+        }},
+        exp_err = 'multikey path is unsupported',
+    },
     {
         'Success case; one part',
         parts = {{