From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp61.i.mail.ru (smtp61.i.mail.ru [217.69.128.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 8ADE243D679 for ; Fri, 25 Oct 2019 00:16:35 +0300 (MSK) From: Vladislav Shpilevoy Date: Thu, 24 Oct 2019 23:21:54 +0200 Message-Id: <62cb7773ed9a10682f8804ae214cff1be2fdda77.1571952007.git.v.shpilevoy@tarantool.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH 1/1] key_def: key_def.new() accept both 'field' and 'fieldno' List-Id: Tarantool development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org Closes #4519 @TarantoolBot document Title: key_def.new() accept both 'field' and 'fieldno' Before the patch key_def.new() took an index part array as it is returned in .parts: each part should include 'type', 'fieldno', and what else .parts element contains. But it was not possible to create a key_def from an index definition - the array passed to .create_index() 'parts' argument. Because key_def.new() didn't recognize 'field' option. That might be useful, when a key_def is needed on a remote client, where a space object and its indexes do not exist. And it would be strange to force a user to create them just so as he would be able to access .space.. index..parts As well as it would be crutchy to make a user manually replace 'field' with 'fieldno' in its index definition just to create a key_def. Additionally, an ability to pass an index definition to a key_def constructor makes the API more symmetric. Note, that it still is not 100% symmetric, because a user can't pass field names to the key_def constructor. A space is needed for that anyway. --- Branch: https://github.com/tarantool/tarantool/tree/gerold103/gh-4519-key_def-conform-create_index Issue: https://github.com/tarantool/tarantool/issues/4519 src/box/lua/key_def.c | 22 ++++++++++++++++++++-- test/box-tap/key_def.test.lua | 23 +++++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/box/lua/key_def.c b/src/box/lua/key_def.c index 3a3a5ec0c..d8f96162d 100644 --- a/src/box/lua/key_def.c +++ b/src/box/lua/key_def.c @@ -88,8 +88,26 @@ luaT_key_def_set_part(struct lua_State *L, struct key_part_def *part, lua_pushstring(L, "fieldno"); lua_gettable(L, -2); if (lua_isnil(L, -1)) { - diag_set(IllegalParams, "fieldno must not be nil"); - return -1; + lua_pop(L, 1); + /* + * 'field' is an alias for fieldno to support the + * same parts format as is used in + * .create_index() in Lua. + */ + lua_getfield(L, -1, "field"); + if (lua_isnil(L, -1)) { + diag_set(IllegalParams, + "fieldno or field must not be nil"); + return -1; + } + } else { + lua_getfield(L, -2, "field"); + if (! lua_isnil(L, -1)) { + diag_set(IllegalParams, + "Conflicting options: fieldno and field"); + return -1; + } + lua_pop(L, 1); } /* * Transform one-based Lua fieldno to zero-based diff --git a/test/box-tap/key_def.test.lua b/test/box-tap/key_def.test.lua index 4b468a696..d7dbf5b88 100755 --- a/test/box-tap/key_def.test.lua +++ b/test/box-tap/key_def.test.lua @@ -154,6 +154,29 @@ local key_def_new_cases = { }}, exp_err = nil, }, + -- + -- gh-4519: key_def should allow the same options as + -- .create_index(). That is, a field number + -- should be allowed to be specified as `field`, not only + -- `fieldno`. + -- + { + 'Success case; `field` is alias to `fieldno`', + parts = {{ + field = 1, + type = 'unsigned' + }}, + exp_err = nil, + }, + { + 'Field and fieldno can not be set both', + parts = {{ + field = 1, + fieldno = 1, + type = 'unsigned' + }}, + exp_err = 'Conflicting options: fieldno and field', + } } local test = tap.test('key_def') -- 2.21.0 (Apple Git-122)