* [Tarantool-patches] [PATCH 1/1] key_def: key_def.new() accept both 'field' and 'fieldno'
@ 2019-10-24 21:21 Vladislav Shpilevoy
2019-10-24 23:28 ` Alexander Turenko
0 siblings, 1 reply; 2+ messages in thread
From: Vladislav Shpilevoy @ 2019-10-24 21:21 UTC (permalink / raw)
To: tarantool-patches
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)
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [Tarantool-patches] [PATCH 1/1] key_def: key_def.new() accept both 'field' and 'fieldno'
2019-10-24 21:21 [Tarantool-patches] [PATCH 1/1] key_def: key_def.new() accept both 'field' and 'fieldno' Vladislav Shpilevoy
@ 2019-10-24 23:28 ` Alexander Turenko
0 siblings, 0 replies; 2+ messages in thread
From: Alexander Turenko @ 2019-10-24 23:28 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches, tarantool-patches
LGTM.
WBR, Alexander Turenko.
On Thu, Oct 24, 2019 at 11:21:54PM +0200, Vladislav Shpilevoy wrote:
> 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
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-10-24 23:28 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-24 21:21 [Tarantool-patches] [PATCH 1/1] key_def: key_def.new() accept both 'field' and 'fieldno' Vladislav Shpilevoy
2019-10-24 23:28 ` Alexander Turenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox