Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko <alexander.turenko@tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org,
	Alexander Turenko <alexander.turenko@tarantool.org>
Subject: [Tarantool-patches] [PATCH v2 06/15] WIP: module api/lua: add luaT_tuple_encode()
Date: Sun, 11 Oct 2020 15:57:39 +0300	[thread overview]
Message-ID: <d5e4b44239540dcedae6df1304ce35db45887918.1602420460.git.alexander.turenko@tarantool.org> (raw)
In-Reply-To: <cover.1602420460.git.alexander.turenko@tarantool.org>

It is the same as luaT_tuple_new(), but returns raw MsgPack data (not
<box_tuple_t>) allocated on the box region.

The reason to expose this function is to provide ability to use
box_tuple_compare_with_key() function from an external module for a key
passed as a Lua table. The compare function has the following signature:

 | API_EXPORT int
 | box_tuple_compare_with_key(box_tuple_t *tuple_a, const char *key_b,
 |                            box_key_def_t *key_def);

The second parameter is a key encoded as an MsgPack array, not a tuple
structure. So luaT_tuple_new() is not applicable here (it is not
worthful to create a tuple structure if we need just MsgPack data).

Some complexity was introduced to support encoding on the Lua shared
buffer and the box region both. The motivation is the following:

- luaT_tuple_encode() is exposed with encoding to the box region,
  because it is more usual to the module API. In particular a user of
  the API able to control when the tuple data should be released.
- Encoding to the Lua shared buffer is kept internally, because there is
  no strong reason to change it to the box region for box.tuple.new().

Part of #5273

XXX: Try to get rid of luaT_mpstream_init_*() functions: call
mpstream_init() outside of the protected section with changed error
handler.
---
 src/box/lua/tuple.c              |  99 ++++++++++++++++++++++----
 src/box/lua/tuple.h              |  17 +++++
 src/exports.h                    |   1 +
 test/app-tap/module_api.c        | 115 +++++++++++++++++++++++++++++++
 test/app-tap/module_api.test.lua |   2 +-
 5 files changed, 220 insertions(+), 14 deletions(-)

diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c
index 18cfef979..8e2255a2b 100644
--- a/src/box/lua/tuple.c
+++ b/src/box/lua/tuple.c
@@ -100,10 +100,31 @@ luaT_istuple(struct lua_State *L, int narg)
 	return *(struct tuple **) data;
 }
 
+/* {{{ Encode a Lua table as an MsgPack array */
+
+/*
+ * A lot of functions are there, however the task per se looks
+ * simple. Reasons are the following.
+ *
+ * 1. box.tuple.new() supports two parameters conventions.
+ *    <luaT_tuple_encode_values>() implements the old API.
+ * 2. Serializer from Lua to MsgPack may raise a Lua error,
+ *    so it should be run under pcall. The dangerous code is
+ *    encapsulated into <luaT_tuple_encode_table>().
+ * 3. In particular <mpstream_init>() may raise an error in case
+ *    of OOM, so it also is run under pcall.
+ * 4. box.tuple.new() and <luaT_tuple_new>() use shared Lua ibuf
+ *    under the hood (because there is no strong reason to change
+ *    it), while <luaT_tuple_encode>() uses the box region
+ *    (because it is usual for the module API).
+ */
+
 /**
  * Encode a Lua values on a Lua stack as an MsgPack array.
  *
  * Raise a Lua error when encoding fails.
+ *
+ * Helper for <lbox_tuple_new>().
  */
 static int
 luaT_tuple_encode_values(struct lua_State *L)
@@ -122,6 +143,22 @@ luaT_tuple_encode_values(struct lua_State *L)
 	return 0;
 }
 
+typedef void luaT_mpstream_init_f(struct mpstream *stream, struct lua_State *L);
+
+static void
+luaT_mpstream_init_lua_ibuf(struct mpstream *stream, struct lua_State *L)
+{
+	mpstream_init(stream, tarantool_lua_ibuf, ibuf_reserve_cb,
+		      ibuf_alloc_cb, luamp_error, L);
+}
+
+static void
+luaT_mpstream_init_box_region(struct mpstream *stream, struct lua_State *L)
+{
+	mpstream_init(stream, &fiber()->gc, region_reserve_cb, region_alloc_cb,
+		      luamp_error, L);
+}
+
 /**
  * Encode a Lua table or a tuple as MsgPack.
  *
@@ -132,25 +169,26 @@ luaT_tuple_encode_values(struct lua_State *L)
 static int
 luaT_tuple_encode_table(struct lua_State *L)
 {
-	struct ibuf *buf = tarantool_lua_ibuf;
-	ibuf_reset(buf);
 	struct mpstream stream;
-	mpstream_init(&stream, buf, ibuf_reserve_cb, ibuf_alloc_cb, luamp_error,
-		      L);
-	luamp_encode_tuple(L, &tuple_serializer, &stream, 1);
+	luaT_mpstream_init_f *luaT_mpstream_init_f = lua_topointer(L, 1);
+	luaT_mpstream_init_f(&stream, L);
+	luamp_encode_tuple(L, &tuple_serializer, &stream, 2);
 	mpstream_flush(&stream);
 	return 0;
 }
 
-static char *
-luaT_tuple_encode_on_lua_ibuf(struct lua_State *L, int idx,
-			      size_t *tuple_len_ptr)
+/**
+ * Encode a Lua table / tuple to given mpstream.
+ */
+static int
+luaT_tuple_encode_helper(struct lua_State *L, int idx,
+			 luaT_mpstream_init_f *luaT_mpstream_init_f)
 {
 	assert(idx != 0);
 	if (!lua_istable(L, idx) && !luaT_istuple(L, idx)) {
 		diag_set(IllegalParams, "A tuple or a table expected, got %s",
 			 lua_typename(L, lua_type(L, idx)));
-		return NULL;
+		return -1;
 	}
 
 	int top = lua_gettop(L);
@@ -163,18 +201,53 @@ luaT_tuple_encode_on_lua_ibuf(struct lua_State *L, int idx,
 	lua_rawgeti(L, LUA_REGISTRYINDEX, luaT_tuple_encode_table_ref);
 	assert(lua_isfunction(L, -1));
 
+	lua_pushlightuserdata(L, luaT_mpstream_init_f);
 	lua_pushvalue(L, idx);
 
-	int rc = luaT_call(L, 1, 0);
+	int rc = luaT_call(L, 2, 0);
 	lua_settop(L, top);
-	if (rc != 0)
+	return rc == 0 ? 0 : -1;
+}
+
+/**
+ * Encode a Lua table / tuple to Lua shared ibuf.
+ */
+static char *
+luaT_tuple_encode_on_lua_ibuf(struct lua_State *L, int idx,
+			      size_t *tuple_len_ptr)
+{
+	struct ibuf *buf = tarantool_lua_ibuf;
+	ibuf_reset(buf);
+	if (luaT_tuple_encode_helper(L, idx, luaT_mpstream_init_lua_ibuf) != 0)
 		return NULL;
+	if (tuple_len_ptr != NULL)
+		*tuple_len_ptr = ibuf_used(buf);
+	return buf->buf;
+}
 
+/**
+ * Encode a Lua table / tuple to box region.
+ */
+char *
+luaT_tuple_encode(struct lua_State *L, int idx, size_t *tuple_len_ptr)
+{
+	struct region *region = &fiber()->gc;
+	size_t region_svp = region_used(region);
+	if (luaT_tuple_encode_helper(L, idx, luaT_mpstream_init_box_region) != 0)
+		return NULL;
+	size_t tuple_len = region_used(region) - region_svp;
 	if (tuple_len_ptr != NULL)
-		*tuple_len_ptr = ibuf_used(tarantool_lua_ibuf);
-	return tarantool_lua_ibuf->buf;
+		*tuple_len_ptr = tuple_len;
+	char *tuple_data = region_join(region, tuple_len);
+	if (tuple_data == NULL) {
+		diag_set(OutOfMemory, tuple_len, "region", "tuple data");
+		return NULL;
+	}
+	return tuple_data;
 }
 
+/* }}} Encode a Lua table as an MsgPack array */
+
 struct tuple *
 luaT_tuple_new(struct lua_State *L, int idx, box_tuple_format_t *format)
 {
diff --git a/src/box/lua/tuple.h b/src/box/lua/tuple.h
index 6787d1afe..aadcf7f59 100644
--- a/src/box/lua/tuple.h
+++ b/src/box/lua/tuple.h
@@ -31,6 +31,7 @@
  * SUCH DAMAGE.
  */
 #include <stddef.h>
+#include "trivia/util.h"
 
 #if defined(__cplusplus)
 extern "C" {
@@ -79,6 +80,22 @@ luaT_pushtuple(struct lua_State *L, box_tuple_t *tuple);
 box_tuple_t *
 luaT_istuple(struct lua_State *L, int idx);
 
+/**
+ * Encode a table or a tuple on the Lua stack as an MsgPack array.
+ *
+ * @param L              Lua state.
+ * @param idx            Acceptable index on the Lua stack.
+ * @param tuple_len_ptr  Where to store tuple data size in bytes
+ *                       (or NULL).
+ *
+ * The storage for data is allocated on the box region. A caller
+ * should call <box_region_truncate>() to release the data.
+ *
+ * In case of an error set a diag and return NULL.
+ */
+API_EXPORT char *
+luaT_tuple_encode(struct lua_State *L, int idx, size_t *tuple_len_ptr);
+
 /** \endcond public */
 
 /**
diff --git a/src/exports.h b/src/exports.h
index 01c1aa83e..7bdba5693 100644
--- a/src/exports.h
+++ b/src/exports.h
@@ -406,6 +406,7 @@ EXPORT(luaT_istuple)
 EXPORT(luaT_pushtuple)
 EXPORT(luaT_state)
 EXPORT(luaT_tolstring)
+EXPORT(luaT_tuple_encode)
 EXPORT(mp_char2escape)
 EXPORT(mp_decode_double)
 EXPORT(mp_decode_extl)
diff --git a/test/app-tap/module_api.c b/test/app-tap/module_api.c
index 928c3a368..9b182144a 100644
--- a/test/app-tap/module_api.c
+++ b/test/app-tap/module_api.c
@@ -555,6 +555,120 @@ test_box_region(struct lua_State *L)
 
 /* }}} test_box_region */
 
+/* {{{ test_tuple_encode */
+
+static void
+check_tuple_data(char *tuple_data, size_t tuple_size, int retvals)
+{
+	assert(tuple_size == 4);
+	assert(tuple_data != NULL);
+	assert(!strncmp(tuple_data, "\x93\x01\x02\x03", 4));
+	assert(retvals == 0);
+}
+
+static void
+check_encode_error(char *tuple_data, int retvals, const char *exp_err_type,
+		   const char *exp_err_msg)
+{
+	assert(tuple_data == NULL);
+	box_error_t *e = box_error_last();
+	assert(!strcmp(box_error_type(e), exp_err_type));
+	assert(!strcmp(box_error_message(e), exp_err_msg));
+	assert(retvals == 0);
+}
+
+/**
+ * Encode a Lua table or a tuple into a tuple.
+ *
+ * Similar to <luaT_tuple_new>() unit test.
+ */
+static int
+test_tuple_encode(struct lua_State *L)
+{
+	int top;
+	char *tuple_data;
+	size_t tuple_size;
+
+	size_t region_svp = box_region_used();
+
+	/*
+	 * Case: a Lua table on idx == -2 as an input.
+	 */
+
+	/* Prepare the Lua stack. */
+	luaL_loadstring(L, "return {1, 2, 3}");
+	lua_call(L, 0, 1);
+	lua_pushnil(L);
+
+	/* Create and check a tuple. */
+	top = lua_gettop(L);
+	tuple_data = luaT_tuple_encode(L, -2, &tuple_size);
+	check_tuple_data(tuple_data, tuple_size, lua_gettop(L) - top);
+
+	/* Clean up. */
+	lua_pop(L, 2);
+	assert(lua_gettop(L) == 0);
+
+	/*
+	 * Case: a tuple on idx == -1 as an input.
+	 */
+
+	/* Prepare the Lua stack. */
+	luaL_loadstring(L, "return box.tuple.new({1, 2, 3})");
+	lua_call(L, 0, 1);
+
+	/* Create and check a tuple. */
+	top = lua_gettop(L);
+	tuple_data = luaT_tuple_encode(L, -1, &tuple_size);
+	check_tuple_data(tuple_data, tuple_size, lua_gettop(L) - top);
+
+	/* Clean up. */
+	lua_pop(L, 1);
+	assert(lua_gettop(L) == 0);
+
+	/*
+	 * Case: a Lua object of an unexpected type.
+	 */
+
+	/* Prepare the Lua stack. */
+	lua_pushinteger(L, 42);
+
+	/* Try to encode and check for the error. */
+	top = lua_gettop(L);
+	tuple_data = luaT_tuple_encode(L, -1, &tuple_size);
+	check_encode_error(tuple_data, lua_gettop(L) - top, "IllegalParams",
+			   "A tuple or a table expected, got number");
+
+	/* Clean up. */
+	lua_pop(L, 1);
+	assert(lua_gettop(L) == 0);
+
+	/*
+	 * Case: unserializable item within a Lua table.
+	 *
+	 * The function should not raise a Lua error.
+	 */
+	luaL_loadstring(L, "return {function() end}");
+	lua_call(L, 0, 1);
+
+	/* Try to encode and check for the error. */
+	top = lua_gettop(L);
+	tuple_data = luaT_tuple_encode(L, -1, &tuple_size);
+	check_encode_error(tuple_data, lua_gettop(L) - top, "LuajitError",
+			   "unsupported Lua type 'function'");
+
+	/* Clean up. */
+	lua_pop(L, 1);
+	assert(lua_gettop(L) == 0);
+
+	box_region_truncate(region_svp);
+
+	lua_pushboolean(L, 1);
+	return 1;
+}
+
+/* }}} test_tuple_encode */
+
 LUA_API int
 luaopen_module_api(lua_State *L)
 {
@@ -585,6 +699,7 @@ luaopen_module_api(lua_State *L)
 		{"iscallable", test_iscallable},
 		{"iscdata", test_iscdata},
 		{"test_box_region", test_box_region},
+		{"test_tuple_encode", test_tuple_encode},
 		{NULL, NULL}
 	};
 	luaL_register(L, "module_api", lib);
diff --git a/test/app-tap/module_api.test.lua b/test/app-tap/module_api.test.lua
index 582c6d85a..262e0751c 100755
--- a/test/app-tap/module_api.test.lua
+++ b/test/app-tap/module_api.test.lua
@@ -172,7 +172,7 @@ local function test_iscdata(test, module)
 end
 
 local test = require('tap').test("module_api", function(test)
-    test:plan(26)
+    test:plan(27)
     local status, module = pcall(require, 'module_api')
     test:is(status, true, "module")
     test:ok(status, "module is loaded")
-- 
2.25.0

  parent reply	other threads:[~2020-10-11 12:57 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-11 12:57 [Tarantool-patches] [PATCH v2 00/15] RFC: module api: extend for external key_def Lua module Alexander Turenko
2020-10-11 12:57 ` [Tarantool-patches] [PATCH v2 01/15] module api: get rid of typedef redefinitions Alexander Turenko
2020-10-11 12:57 ` [Tarantool-patches] [PATCH v2 02/15] module api: expose box region Alexander Turenko
2020-10-11 15:26   ` Vladislav Shpilevoy
2020-10-12  6:07     ` Alexander Turenko
2020-10-11 12:57 ` [Tarantool-patches] [PATCH v2 03/15] module api/lua: add luaL_iscdata() function Alexander Turenko
2020-10-11 12:57 ` [Tarantool-patches] [PATCH v2 04/15] lua: factor out tuple encoding from luaT_tuple_new Alexander Turenko
2020-10-11 12:57 ` [Tarantool-patches] [PATCH v2 05/15] lua: don't raise a Lua error from luaT_tuple_new() Alexander Turenko
2020-10-11 15:25   ` Vladislav Shpilevoy
2020-10-12 10:37     ` Alexander Turenko
2020-10-12 13:34       ` Timur Safin
2020-10-14 23:41       ` Vladislav Shpilevoy
2020-10-15 19:43         ` Alexander Turenko
2020-10-15 22:10           ` Vladislav Shpilevoy
2020-10-11 17:47   ` Igor Munkin
2020-10-11 18:08     ` Igor Munkin
2020-10-12 10:37     ` Alexander Turenko
2020-10-12 10:51       ` Igor Munkin
2020-10-12 18:41         ` Alexander Turenko
2020-10-11 12:57 ` Alexander Turenko [this message]
2020-10-11 15:25   ` [Tarantool-patches] [PATCH v2 06/15] WIP: module api/lua: add luaT_tuple_encode() Vladislav Shpilevoy
2020-10-12 10:35     ` Alexander Turenko
2020-10-11 12:57 ` [Tarantool-patches] [PATCH v2 07/15] module api/lua: expose luaT_tuple_new() Alexander Turenko
2020-10-11 15:25   ` Vladislav Shpilevoy
2020-10-12  6:11     ` Alexander Turenko
2020-10-11 12:57 ` [Tarantool-patches] [PATCH v2 08/15] module api/lua: add API_EXPORT to tuple functions Alexander Turenko
2020-10-11 12:57 ` [Tarantool-patches] [PATCH v2 09/15] module api: add API_EXPORT to key_def functions Alexander Turenko
2020-10-11 12:57 ` [Tarantool-patches] [PATCH v2 10/15] module api: add box_key_def_new_v2() Alexander Turenko
2020-10-11 15:25   ` Vladislav Shpilevoy
2020-10-12  7:21     ` Alexander Turenko
2020-10-11 12:57 ` [Tarantool-patches] [PATCH v2 11/15] module api: add box_key_def_dump_parts() Alexander Turenko
2020-10-11 15:25   ` Vladislav Shpilevoy
2020-10-12  6:50     ` Alexander Turenko
2020-10-11 12:57 ` [Tarantool-patches] [PATCH v2 12/15] module api: expose box_key_def_validate_tuple() Alexander Turenko
2020-10-11 12:57 ` [Tarantool-patches] [PATCH v2 13/15] WIP: module api: expose box_key_def_merge() Alexander Turenko
2020-10-11 12:57 ` [Tarantool-patches] [PATCH v2 14/15] WIP: module api: expose box_key_def_extract_key() Alexander Turenko
2020-10-11 12:57 ` [Tarantool-patches] [PATCH v2 15/15] WIP: module api: add box_key_def_validate_key() 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=d5e4b44239540dcedae6df1304ce35db45887918.1602420460.git.alexander.turenko@tarantool.org \
    --to=alexander.turenko@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v2 06/15] WIP: module api/lua: add luaT_tuple_encode()' \
    /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