Tarantool development patches archive
 help / color / mirror / Atom feed
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)

             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