From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [tarantool-patches] Re: [PATCH v5 4/4] box: introduce multikey indexes in memtx References: <20190506154603.txsxk2hsyshdqtdb@esperanza> <88b86a7c-686d-30ac-fe75-f892381e96a4@tarantool.org> <20190507081103.fnzqguaeev4hxrlt@esperanza> From: Kirill Shcherbatov 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 List-ID: >> + 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 = {{