Re: [PATCH v2 3/6] lua: add luaT_newtuple()

[Alexander Turenko <alexander.turenko@tarantool.org>]

List of changes:

* Changed the name: luaT_newtuple() -> luaT_tuple_new().
* Fix an indentation and grammar typos.
* Extracted an optimization to a separate commit.
* Added a unit test.

Answered inline. Still have unresolved questions (naming and error
handling).

The new version (two commits) is at bottom of the email.

WBR, Alexander Turenko.

On Thu, Jan 10, 2019 at 03:44:46PM +0300, Vladimir Davydov wrote:
> On Wed, Jan 09, 2019 at 11:20:11PM +0300, Alexander Turenko wrote:
> > The function allows to create a tuple with specific tuple format in C
> > code using a Lua table, an another tuple or objects on a Lua stack.
> > 
> > Needed for #3276.
> > ---
> >  src/box/lua/tuple.c | 91 +++++++++++++++++++++++++++++++++------------
> >  src/box/lua/tuple.h | 15 ++++++++
> >  2 files changed, 83 insertions(+), 23 deletions(-)
> 
> Although a test would be nice to have, I guess we can live without it,
> because the new function is tested indirectly via lbox_tuple_new().

There are untested cases: using a tuple as an input, using a non-default
format, an error in case of an unexpected type. 1st is possible with
box.tuple.new(), but is not tested. 2nd and 3rd are not possible with
box.tuple.new().

I have added a unit test for all usage cases of luaT_tuple_new() w/o
varying a tuple, which is done in box/tuple.test.lua. So now all cases
are covered.

> 
> > 
> > diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c
> > index 1867f810f..7e9ad89fe 100644
> > --- a/src/box/lua/tuple.c
> > +++ b/src/box/lua/tuple.c
> > @@ -92,6 +92,65 @@ luaT_istuple(struct lua_State *L, int narg)
> >  	return *(struct tuple **) data;
> >  }
> >  
> > +struct tuple *
> > +luaT_newtuple(struct lua_State *L, int idx, box_tuple_format_t *format)
> 
> I looked at the Lua reference manual and realized that they usually call
> a function lua_newsomething if it creates an object on Lua stack. So I
> guess we'd better rename it to luaT_tuple_new() to avoid confusion.

Renamed, but I'm tentative about this change.

We'll have the following convention:

* lua{,L,T}_foo_new(): read from a Lua stack, return a pointer, push an
  error to a Lua stack (errors is the open question, see below);
* lua{,L,T}_newfoo(): read from a Lua stack, push on a Lua stack, raise
  an error;
* lbox_foo_new(): the same as previous, but exposed into Lua via a
  module / instance fields.

Don't sure I'll able to remember this convention: a name does not much
say about a behaviour. I would stick with luaT_new* or introduce some
more explicit naming for a function to grab a definition from lua and
create an object in C.

Check / to does not fit good:

* lua{L,T}_checkfoo() -> struct foo *
  - Like luaL_checkstring().
  - But it looks as if it can raise an exception.
  - But it looks as if we'll unwrap a pointer from a cdata.
* lua{L,T}_tofoo() -> struct foo *
  - Like lua_tolstring().
  - But it looks as if we'll unwrap a pointer from a cdata.

Another possible variants (in order of increasing weirdness for me):

* lua{L,T}_makefoo();
* lua{L,T}_createfoo();
* lua{L,T}_parsefoo();
* lua{L,T}_newfoo_c();
* lua{L,T}_constructfoo();
* lua{L,T}_foo_new_fromlua();

BTW, '_c' postfix can suggest that a function cannot raise an error.

There are lua{,L}_new* functions that returns a pointer:

* luaL_newstate(void) -> struct lua_State *
* lua_newthread(struct lua_State *) -> struct lua_State *
* lua_newuserdata(struct lua_State *, size_t) -> void *

Maybe it is not criminal to do the same under luaT_newtuple()?

We also can use luaT_create*, I found only one function with such
naming: lua_createtable().

I think the general approach could be the following: use
lua{L,T}_newfoo() or lua{L,T}_createfoo() if the former one is busy.

luaT_maketuple() and lua{L,T}_newtuple_c() looks appropriate for me too.
Latter a bit ugly, but '_c' is informative (no exceptions).

> 
> > +{
> > +	struct tuple *tuple;
> > +
> > +	if (idx == 0 || lua_istable(L, idx)) {
> > +		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);
> 
> Nit: bad indentation.

Fixed.

> 
> > +		if (idx == 0) {
> > +			/*
> > +			 * Create the tuple from lua stack
> > +			 * objects.
> > +			 */
> > +			int argc = lua_gettop(L);
> > +			mpstream_encode_array(&stream, argc);
> > +			for (int k = 1; k <= argc; ++k) {
> > +				luamp_encode(L, luaL_msgpack_default, &stream,
> > +					     k);
> > +			}
> > +		} else {
> > +			/* Create the tuple from a Lua table. */
> > +			luamp_encode_tuple(L, luaL_msgpack_default, &stream,
> > +					   idx);
> > +		}
> > +		mpstream_flush(&stream);
> > +		tuple = box_tuple_new(format, buf->buf,
> > +				      buf->buf + ibuf_used(buf));
> > +		if (tuple == NULL) {
> > +			luaT_pusherror(L, diag_last_error(diag_get()));
> 
> Why not simply throw the error with luaT_error()? Other similar
> functions throw an error, not just push it to the stack.

Because in a caller I need to perform clean up before reraise it.
lua_pcall() would be expensive.

luaT_tuple_new() is used in a table and an iterator source in next().
next() is used in two places: merger_next() where I just pop and raise
the error and in merger_source_new() to acquire a first tuple. If an
error occurs here I need to free the newly created source, then in
merger_state_new() free newly created merger_state with all successfully
created sources (it also perform needed unref's). I cannot allow any
function called on this path to raise an error.

I can implement reference counting of all that objects and free them in
the next call to merger (some kind of simple gc), but this way looks as
overengineered.

Mine errors are strings and it is convenient to create them with
lua_pushfstring() or push memory errors with luaT_pusherror().

There are two variants how to avoid raising an error:

* lua_pushfstring();
* diag_set().

Latter seems to be more native for tarantool. I would use something like
XlogError: printf-style format string + vararg. But I doubt how should I
name such class? ContractError (most of them are about bad args)? There
is also unexpected buffer end, it is more RuntimeError.

I dislike the idea to copy XlogError code under another name. Maybe we
can implement a general class for such errors and inherit it in
XlogError, ContractError and RuntimeError?

I choose pushing to stack, because it is the most simple solution, and
forget to discuss it with you. My bad.

Please, give me some pointer here.

> 
> > +			return NULL;
> > +		}
> > +		ibuf_reinit(tarantool_lua_ibuf);
> > +		return tuple;
> > +	}
> > +
> > +	tuple = luaT_istuple(L, idx);
> > +	if (tuple == NULL) {
> > +		lua_pushfstring(L, "A tuple or a table expected, got %s",
> > +				lua_typename(L, lua_type(L, -1)));
> > +		return NULL;
> > +	}
> > +
> > +	/*
> > +	 * Create the new tuple with the necessary format from
> 
> Nit: a new tuple

Fixed.

> 
> > +	 * the another tuple.
> 
> Nit: 'the' is redundant.

Fixed.

> 
> > +	 */
> > +	const char *tuple_beg = tuple_data(tuple);
> > +	const char *tuple_end = tuple_beg + tuple->bsize;
> > +	tuple = box_tuple_new(format, tuple_beg, tuple_end);
> > +	if (tuple == NULL) {
> > +		luaT_pusherror(L, diag_last_error(diag_get()));
> > +		return NULL;
> > +	}
> > +	return tuple;
> 
> I see that you reworked the original code so as to avoid tuple data
> copying in case a new tuple is created from another tuple. That's OK,
> but I think that it should've been done in a separate patch.

Extracted.

Copying is still performed, but it is one mempcy() in
runtime_tuple_new() and does not involve parsing.

> 
> > +}
> > +
> >  int
> >  lbox_tuple_new(lua_State *L)
> >  {
> > @@ -100,33 +159,19 @@ lbox_tuple_new(lua_State *L)
> >  		lua_newtable(L); /* create an empty tuple */
> >  		++argc;
> >  	}
> > -	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);
> > -
> > -	if (argc == 1 && (lua_istable(L, 1) || luaT_istuple(L, 1))) {
> > -		/* New format: box.tuple.new({1, 2, 3}) */
> > -		luamp_encode_tuple(L, luaL_msgpack_default, &stream, 1);
> > -	} else {
> > -		/* Backward-compatible format: box.tuple.new(1, 2, 3). */
> > -		mpstream_encode_array(&stream, argc);
> > -		for (int k = 1; k <= argc; ++k) {
> > -			luamp_encode(L, luaL_msgpack_default, &stream, k);
> > -		}
> > -	}
> > -	mpstream_flush(&stream);
> > -
> > +	/*
> > +	 * Use backward-compatible parameters format:
> > +	 * box.tuple.new(1, 2, 3) (idx == 0), or the new one:
> > +	 * box.tuple.new({1, 2, 3}) (idx == 1).
> > +	 */
> > +	int idx = argc == 1 && (lua_istable(L, 1) ||
> > +		luaT_istuple(L, 1));
> >  	box_tuple_format_t *fmt = box_tuple_format_default();
> > -	struct tuple *tuple = box_tuple_new(fmt, buf->buf,
> > -					   buf->buf + ibuf_used(buf));
> > +	struct tuple *tuple = luaT_newtuple(L, idx, fmt);
> >  	if (tuple == NULL)
> > -		return luaT_error(L);
> > +		return lua_error(L);
> >  	/* box_tuple_new() doesn't leak on exception, see public API doc */
> >  	luaT_pushtuple(L, tuple);
> > -	ibuf_reinit(tarantool_lua_ibuf);
> >  	return 1;
> >  }
> >  
> > diff --git a/src/box/lua/tuple.h b/src/box/lua/tuple.h
> > index 5d7062eb8..3319b951e 100644
> > --- a/src/box/lua/tuple.h
> > +++ b/src/box/lua/tuple.h
> > @@ -41,6 +41,8 @@ typedef struct tuple box_tuple_t;
> >  struct lua_State;
> >  struct mpstream;
> >  struct luaL_serializer;
> > +struct tuple_format;
> > +typedef struct tuple_format box_tuple_format_t;
> >  
> >  /** \cond public */
> >  
> > @@ -66,6 +68,19 @@ luaT_istuple(struct lua_State *L, int idx);
> >  
> >  /** \endcond public */
> >  
> > +/**
> > + * Create the new tuple with specific format from a Lua table, a
> 
> Nit: a new tuple

Fixed.

> 
> > + * tuple or objects on the lua stack.
> 
> Nit: comma before 'or' is missing ;-)

An oxford comma is not mandatory :)

Added.

> 
> > + *
> > + * Set idx to zero to create the new tuple from objects on the lua
> > + * stack.
> > + *
> > + * In case of an error push the error message to the Lua stack and
> > + * return NULL.
> > + */
> > +struct tuple *
> > +luaT_newtuple(struct lua_State *L, int idx, box_tuple_format_t *format);
> > +
> >  int
> >  lbox_tuple_new(struct lua_State *L);

----

commit 1c49e0cc9dd96da7f4ac45797b427b19168c47a4
Author: Alexander Turenko <alexander.turenko@tarantool.org>
Date:   Mon Jan 7 16:52:33 2019 +0300

    lua: add luaT_tuple_new()
    
    The function allows to create a tuple with specific tuple format in C
    code using a Lua table, another tuple, or objects on a Lua stack.
    
    Needed for #3276.

diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c
index 1867f810f..a2a06a214 100644
--- a/src/box/lua/tuple.c
+++ b/src/box/lua/tuple.c
@@ -92,41 +92,66 @@ luaT_istuple(struct lua_State *L, int narg)
 	return *(struct tuple **) data;
 }
 
-int
-lbox_tuple_new(lua_State *L)
+struct tuple *
+luaT_tuple_new(struct lua_State *L, int idx, box_tuple_format_t *format)
 {
-	int argc = lua_gettop(L);
-	if (argc < 1) {
-		lua_newtable(L); /* create an empty tuple */
-		++argc;
+	if (idx != 0 && !lua_istable(L, idx) && !luaT_istuple(L, idx)) {
+		lua_pushfstring(L, "A tuple or a table expected, got %s",
+				lua_typename(L, lua_type(L, idx)));
+		return NULL;
 	}
-	struct ibuf *buf = tarantool_lua_ibuf;
 
+	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);
-
-	if (argc == 1 && (lua_istable(L, 1) || luaT_istuple(L, 1))) {
-		/* New format: box.tuple.new({1, 2, 3}) */
-		luamp_encode_tuple(L, luaL_msgpack_default, &stream, 1);
-	} else {
-		/* Backward-compatible format: box.tuple.new(1, 2, 3). */
+	if (idx == 0) {
+		/*
+		 * Create the tuple from lua stack
+		 * objects.
+		 */
+		int argc = lua_gettop(L);
 		mpstream_encode_array(&stream, argc);
 		for (int k = 1; k <= argc; ++k) {
 			luamp_encode(L, luaL_msgpack_default, &stream, k);
 		}
+	} else {
+		/* Create the tuple from a Lua table. */
+		luamp_encode_tuple(L, luaL_msgpack_default, &stream, idx);
 	}
 	mpstream_flush(&stream);
+	struct tuple *tuple = box_tuple_new(format, buf->buf,
+					    buf->buf + ibuf_used(buf));
+	if (tuple == NULL) {
+		luaT_pusherror(L, diag_last_error(diag_get()));
+		return NULL;
+	}
+	ibuf_reinit(tarantool_lua_ibuf);
+	return tuple;
+}
 
+int
+lbox_tuple_new(lua_State *L)
+{
+	int argc = lua_gettop(L);
+	if (argc < 1) {
+		lua_newtable(L); /* create an empty tuple */
+		++argc;
+	}
+	/*
+	 * Use backward-compatible parameters format:
+	 * box.tuple.new(1, 2, 3) (idx == 0), or the new one:
+	 * box.tuple.new({1, 2, 3}) (idx == 1).
+	 */
+	int idx = argc == 1 && (lua_istable(L, 1) ||
+		luaT_istuple(L, 1));
 	box_tuple_format_t *fmt = box_tuple_format_default();
-	struct tuple *tuple = box_tuple_new(fmt, buf->buf,
-					   buf->buf + ibuf_used(buf));
+	struct tuple *tuple = luaT_tuple_new(L, idx, fmt);
 	if (tuple == NULL)
-		return luaT_error(L);
+		return lua_error(L);
 	/* box_tuple_new() doesn't leak on exception, see public API doc */
 	luaT_pushtuple(L, tuple);
-	ibuf_reinit(tarantool_lua_ibuf);
 	return 1;
 }
 
diff --git a/src/box/lua/tuple.h b/src/box/lua/tuple.h
index 5d7062eb8..f8c8ccf1c 100644
--- a/src/box/lua/tuple.h
+++ b/src/box/lua/tuple.h
@@ -41,6 +41,8 @@ typedef struct tuple box_tuple_t;
 struct lua_State;
 struct mpstream;
 struct luaL_serializer;
+struct tuple_format;
+typedef struct tuple_format box_tuple_format_t;
 
 /** \cond public */
 
@@ -66,6 +68,19 @@ luaT_istuple(struct lua_State *L, int idx);
 
 /** \endcond public */
 
+/**
+ * Create a new tuple with specific format from a Lua table, a
+ * tuple, or objects on the lua stack.
+ *
+ * Set idx to zero to create the new tuple from objects on the lua
+ * stack.
+ *
+ * In case of an error push the error message to the Lua stack and
+ * return NULL.
+ */
+struct tuple *
+luaT_tuple_new(struct lua_State *L, int idx, box_tuple_format_t *format);
+
 int
 lbox_tuple_new(struct lua_State *L);
 
diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt
index c2c45a4b8..4cedc09a4 100644
--- a/test/unit/CMakeLists.txt
+++ b/test/unit/CMakeLists.txt
@@ -138,10 +138,15 @@ add_executable(histogram.test histogram.c)
 target_link_libraries(histogram.test stat unit)
 add_executable(ratelimit.test ratelimit.c)
 target_link_libraries(ratelimit.test unit)
+
 add_executable(luaL_iterator.test luaL_iterator.c)
 target_link_libraries(luaL_iterator.test unit server core misc
     ${CURL_LIBRARIES} ${LIBYAML_LIBRARIES} ${READLINE_LIBRARIES}
     ${ICU_LIBRARIES} ${LUAJIT_LIBRARIES})
+add_executable(luaT_tuple_new.test luaT_tuple_new.c)
+target_link_libraries(luaT_tuple_new.test unit box server core misc
+    ${CURL_LIBRARIES} ${LIBYAML_LIBRARIES} ${READLINE_LIBRARIES}
+    ${ICU_LIBRARIES} ${LUAJIT_LIBRARIES})
 
 add_executable(say.test say.c)
 target_link_libraries(say.test core unit)
diff --git a/test/unit/luaT_tuple_new.c b/test/unit/luaT_tuple_new.c
new file mode 100644
index 000000000..07fa1a792
--- /dev/null
+++ b/test/unit/luaT_tuple_new.c
@@ -0,0 +1,174 @@
+#include <string.h>           /* strncmp() */
+#include <lua.h>              /* lua_*() */
+#include <lauxlib.h>          /* luaL_*() */
+#include <lualib.h>           /* luaL_openlibs() */
+#include "unit.h"             /* plan, header, footer, is, ok */
+#include "memory.h"           /* memory_init() */
+#include "fiber.h"            /* fiber_init() */
+#include "small/ibuf.h"       /* struct ibuf */
+#include "box/box.h"          /* box_init() */
+#include "box/tuple.h"        /* box_tuple_format_default() */
+#include "lua/msgpack.h"      /* luaopen_msgpack() */
+#include "box/lua/tuple.h"    /* luaL_iterator_*() */
+
+/*
+ * This test checks all usage cases of luaT_tuple_new():
+ *
+ * * Use with idx == 0 and idx != 0.
+ * * Use with default and non-default formats.
+ * * Use a table and a tuple as an input.
+ * * Use with an unexpected lua type as an input.
+ *
+ * The test does not vary an input table/tuple. This is done in
+ * box/tuple.test.lua.
+ */
+
+extern struct ibuf *tarantool_lua_ibuf;
+
+uint32_t
+min_u32(uint32_t a, uint32_t b)
+{
+	return a < b ? a : b;
+}
+
+void
+check_tuple(const struct tuple *tuple, box_tuple_format_t *format,
+	    int retvals, const char *case_name)
+{
+	uint32_t size;
+	const char *data = tuple_data_range(tuple, &size);
+
+	ok(tuple != NULL, "%s: tuple != NULL", case_name);
+	is(tuple->format_id, tuple_format_id(format),
+	   "%s: check tuple format id", case_name);
+	is(size, 4, "%s: check tuple size", case_name);
+	ok(!strncmp(data, "\x93\x01\x02\x03", min_u32(size, 4)),
+	   "%s: check tuple data", case_name);
+	is(retvals, 0, "%s: check retvals count", case_name);
+}
+
+void check_error(struct lua_State *L, const struct tuple *tuple, int retvals,
+		 const char *case_name)
+{
+	const char *exp_err = "A tuple or a table expected, got number";
+	is(tuple, NULL, "%s: tuple == NULL", case_name);
+	is(retvals, 1, "%s: check retvals count", case_name);
+	is(lua_type(L, -1), LUA_TSTRING, "%s: check error type", case_name);
+	ok(!strcmp(lua_tostring(L, -1), exp_err), "%s: check error message",
+	   case_name);
+}
+
+int
+test_basic(struct lua_State *L)
+{
+	plan(19);
+	header();
+
+	int top;
+	struct tuple *tuple;
+	box_tuple_format_t *default_format = box_tuple_format_default();
+
+	/*
+	 * 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 = luaT_tuple_new(L, -2, default_format);
+	check_tuple(tuple, default_format, lua_gettop(L) - top, "table");
+
+	/* Clean up. */
+	lua_pop(L, 2);
+	assert(lua_gettop(L) == 0);
+
+	/*
+	 * Case: a tuple on idx == -1 as an input.
+	 */
+
+	/* Prepare the Lua stack. */
+	luaT_pushtuple(L, tuple);
+
+	/* Create and check a tuple. */
+	top = lua_gettop(L);
+	tuple = luaT_tuple_new(L, -1, default_format);
+	check_tuple(tuple, default_format, lua_gettop(L) - top, "tuple");
+
+	/* Clean up. */
+	lua_pop(L, 1);
+	assert(lua_gettop(L) == 0);
+
+	/*
+	 * Case: elements on the stack (idx == 0) as an input and
+	 * a non-default format.
+	 */
+
+	/* Prepare the Lua stack. */
+	lua_pushinteger(L, 1);
+	lua_pushinteger(L, 2);
+	lua_pushinteger(L, 3);
+
+	/* Create a new format. */
+	struct key_part_def part;
+	part.fieldno = 0;
+	part.type = FIELD_TYPE_INTEGER;
+	part.coll_id = COLL_NONE;
+	part.is_nullable = false;
+	part.nullable_action = ON_CONFLICT_ACTION_DEFAULT;
+	part.sort_order = SORT_ORDER_ASC;
+	struct key_def *key_def = key_def_new(&part, 1);
+	box_tuple_format_t *another_format = box_tuple_format_new(&key_def, 1);
+	key_def_delete(key_def);
+
+	/* Create and check a tuple. */
+	top = lua_gettop(L);
+	tuple = luaT_tuple_new(L, 0, another_format);
+	check_tuple(tuple, another_format, lua_gettop(L) - top, "objects");
+
+	/* Clean up. */
+	tuple_format_delete(another_format);
+	lua_pop(L, 3);
+	assert(lua_gettop(L) == 0);
+
+	/*
+	 * Case: a lua object of an unexpected type.
+	 */
+
+	/* Prepare the Lua stack. */
+	lua_pushinteger(L, 42);
+
+	/* Try to create and check for the error. */
+	top = lua_gettop(L);
+	tuple = luaT_tuple_new(L, -1, default_format);
+	check_error(L, tuple, lua_gettop(L) - top, "unexpected type");
+
+	/* Clean up. */
+	lua_pop(L, 2);
+	assert(lua_gettop(L) == 0);
+
+	footer();
+	return check_plan();
+}
+
+int
+main()
+{
+	memory_init();
+	fiber_init(fiber_c_invoke);
+
+	ibuf_create(tarantool_lua_ibuf, &cord()->slabc, 16000);
+
+	struct lua_State *L = luaL_newstate();
+	luaL_openlibs(L);
+
+	box_init();
+	box_lua_tuple_init(L);
+	luaopen_msgpack(L);
+	lua_pop(L, 1);
+
+	return test_basic(L);
+}
diff --git a/test/unit/luaT_tuple_new.result b/test/unit/luaT_tuple_new.result
new file mode 100644
index 000000000..110aa68c2
--- /dev/null
+++ b/test/unit/luaT_tuple_new.result
@@ -0,0 +1,22 @@
+1..19
+	*** test_basic ***
+ok 1 - table: tuple != NULL
+ok 2 - table: check tuple format id
+ok 3 - table: check tuple size
+ok 4 - table: check tuple data
+ok 5 - table: check retvals count
+ok 6 - tuple: tuple != NULL
+ok 7 - tuple: check tuple format id
+ok 8 - tuple: check tuple size
+ok 9 - tuple: check tuple data
+ok 10 - tuple: check retvals count
+ok 11 - objects: tuple != NULL
+ok 12 - objects: check tuple format id
+ok 13 - objects: check tuple size
+ok 14 - objects: check tuple data
+ok 15 - objects: check retvals count
+ok 16 - unexpected type: tuple == NULL
+ok 17 - unexpected type: check retvals count
+ok 18 - unexpected type: check error type
+ok 19 - unexpected type: check error message
+	*** test_basic: done ***

----

commit d66cb0e3769b6f21b06f08ffb38ba3e0c6718346
Author: Alexander Turenko <alexander.turenko@tarantool.org>
Date:   Thu Jan 17 23:42:34 2019 +0300

    lua: optimize creation of a tuple from a tuple
    
    Don't parse tuple data, just copy it.

diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c
index a2a06a214..ab861c6d2 100644
--- a/src/box/lua/tuple.c
+++ b/src/box/lua/tuple.c
@@ -95,39 +95,59 @@ luaT_istuple(struct lua_State *L, int narg)
 struct tuple *
 luaT_tuple_new(struct lua_State *L, int idx, box_tuple_format_t *format)
 {
-	if (idx != 0 && !lua_istable(L, idx) && !luaT_istuple(L, idx)) {
+	struct tuple *tuple;
+
+	if (idx == 0 || lua_istable(L, idx)) {
+		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);
+		if (idx == 0) {
+			/*
+			 * Create the tuple from lua stack
+			 * objects.
+			 */
+			int argc = lua_gettop(L);
+			mpstream_encode_array(&stream, argc);
+			for (int k = 1; k <= argc; ++k) {
+				luamp_encode(L, luaL_msgpack_default, &stream,
+					     k);
+			}
+		} else {
+			/* Create the tuple from a Lua table. */
+			luamp_encode_tuple(L, luaL_msgpack_default, &stream,
+					   idx);
+		}
+		mpstream_flush(&stream);
+		tuple = box_tuple_new(format, buf->buf,
+				      buf->buf + ibuf_used(buf));
+		if (tuple == NULL) {
+			luaT_pusherror(L, diag_last_error(diag_get()));
+			return NULL;
+		}
+		ibuf_reinit(tarantool_lua_ibuf);
+		return tuple;
+	}
+
+	tuple = luaT_istuple(L, idx);
+	if (tuple == NULL) {
 		lua_pushfstring(L, "A tuple or a table expected, got %s",
 				lua_typename(L, lua_type(L, idx)));
 		return NULL;
 	}
 
-	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);
-	if (idx == 0) {
-		/*
-		 * Create the tuple from lua stack
-		 * objects.
-		 */
-		int argc = lua_gettop(L);
-		mpstream_encode_array(&stream, argc);
-		for (int k = 1; k <= argc; ++k) {
-			luamp_encode(L, luaL_msgpack_default, &stream, k);
-		}
-	} else {
-		/* Create the tuple from a Lua table. */
-		luamp_encode_tuple(L, luaL_msgpack_default, &stream, idx);
-	}
-	mpstream_flush(&stream);
-	struct tuple *tuple = box_tuple_new(format, buf->buf,
-					    buf->buf + ibuf_used(buf));
+	/*
+	 * Create a new tuple with the necessary format from
+	 * another tuple.
+	 */
+	const char *tuple_beg = tuple_data(tuple);
+	const char *tuple_end = tuple_beg + tuple->bsize;
+	tuple = box_tuple_new(format, tuple_beg, tuple_end);
 	if (tuple == NULL) {
 		luaT_pusherror(L, diag_last_error(diag_get()));
 		return NULL;
 	}
-	ibuf_reinit(tarantool_lua_ibuf);
 	return tuple;
 }