From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: tarantool-patches@dev.tarantool.org Subject: [Tarantool-patches] [PATCH 1/1] key_def: key_def.new() accept both 'field' and 'fieldno' Date: Thu, 24 Oct 2019 23:21:54 +0200 [thread overview] Message-ID: <62cb7773ed9a10682f8804ae214cff1be2fdda77.1571952007.git.v.shpilevoy@tarantool.org> (raw) 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 <index_object>.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 <space_object>.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 <net_box connection>.space.<space_name>. index.<index_name>.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 + * <space_object>.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 + -- <space_object>.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)
next reply other threads:[~2019-10-24 21:16 UTC|newest] Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-10-24 21:21 Vladislav Shpilevoy [this message] 2019-10-24 23:28 ` Alexander Turenko
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=62cb7773ed9a10682f8804ae214cff1be2fdda77.1571952007.git.v.shpilevoy@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH 1/1] key_def: key_def.new() accept both '\''field'\'' and '\''fieldno'\''' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox