[PATCH v1 2/2] netbox: define formats for tuple from netbox

Mergen Imeev imeevma at tarantool.org
Fri Jun 14 15:29:21 MSK 2019


Thank you for review! There will be two new patches below.


On Tue, Jun 11, 2019 at 11:55:11AM +0300, Vladimir Davydov wrote:
> On Mon, Jun 10, 2019 at 01:02:24PM +0300, imeevma at tarantool.org wrote:
> > This patch creates tuple_formats for the tuples obtained through
> > the netbox.
> > 
> > Closes #2978
> 
> Please write a docbot request.
> 
Done:

@TarantoolBot document
Title: Field names for tuples received from net.box

It is possible now to access by field name for tuples received
from net.box. For example:

box.cfg{listen = 3302}
box.schema.user.grant('guest','read, write, execute', 'space')
box.schema.user.grant('guest', 'create', 'space')

box.schema.create_space("named", {format = {{name = "id"}}})
box.space.named:create_index('id', {parts = {{1, 'unsigned'}}})
box.space.named:insert({1})
require('net.box').connect('localhost', 3302).space.named:get(1).id

Result:

tarantool> require('net.box').connect('localhost', 3302).space.named:get(1).id
---
- 1
...

> >  static void
> > -netbox_decode_data(struct lua_State *L, const char **data)
> > +netbox_decode_data(struct lua_State *L, const char **data, uint32_t format_id)
> 
> I'd pass a pointer to tuple_format struct instead of format_id to
> this function.
> 
Done.

> >  {
> > +	(void)format_id;
> 
> This is clearly useless.
> 
Fixed.

> > +	if (lua_gettop(L) != 1 || lua_istable(L, 1) != 1)
> > +		return luaL_error(L, "Bad params!");
> 
> Please print a "Usage: ..." message, like other functions do.
> 
Fixed.

> > +
> > +	uint32_t count = lua_objlen(L, 1);
> > +	if (count == 0) {
> > +		lua_pushinteger(L, box_tuple_format_default()->id);
> 
> Please use tuple_format_runtime instead of box_tuple_format_default(),
> here and everywhere else. The latter is, after all, for modules.
> 
Fixed.

> > +	struct field_def *fields = region_alloc(region, size);
> > +	if (fields == NULL) {
> > +		diag_set(OutOfMemory, size, "region_alloc", "fields");
> > +		return luaT_error(L);
> > +	}
> > +	memset(fields, 0, size);
> 
> Please init each field with field_def_default in the loop below
> instead of zeroing it.
> 
Fixed.

> > +
> > +		lua_pushstring(L, "name");
> > +		lua_gettable(L, -2);
> > +		const char *name = lua_tolstring(L, -1, &len);
> 
> Strictly speaking, the name or type can be absent. The type can also be
> invalid. We should handle those situations gracefully.
> 
Fixed.

> > +		fields[i].name = region_alloc(region, len + 1);
> > +		memcpy(fields[i].name, name, len);
> > +		fields[i].name[len] = '\0';
> > +		lua_pop(L, 1);
> > +		lua_pop(L, 1);
> > +	}
> > +	struct tuple_dictionary *dict = tuple_dictionary_new(fields, count);
> > +	if (dict == NULL) {
> > +		region_truncate(region, region_svp);
> > +		return luaT_error(L);
> > +	}
> > +
> > +	struct tuple_format *format =
> > +		tuple_format_new(&box_tuple_format_default()->vtab, NULL, NULL,
> > +				 0, fields, count, 0, dict, false, false);
> 
> Do we need really need to pass fields to this function? AFAIU tuple
> dictionary would be enough since we only need names.
You are right.

> What happens if a field is nullable?
> 
It works fine, I think:

box.cfg{listen = 3302}
box.schema.user.grant('guest','read, write, execute', 'space')
box.schema.user.grant('guest', 'create', 'space')

box.schema.create_space("named", {format = {{name = "id"}, {name = "nf", is_nullable = true}}})
box.space.named:create_index('id', {parts = {{1, 'unsigned'}}})
box.space.named:insert({1})
box.space.named:insert({2,3})
box.space.named:get(1):tomap()
box.space.named:get(2):tomap()
require('net.box').connect('localhost', 3302).space.named:get(1):tomap()
require('net.box').connect('localhost', 3302).space.named:get(2):tomap()

tarantool> box.space.named:get(1):tomap()
---
- 1: 1
  id: 1
...

tarantool> box.space.named:get(2):tomap()
---
- 1: 2
  2: 3
  nf: 3
  id: 2
...

tarantool> require('net.box').connect('localhost', 3302).space.named:get(1):tomap()
---
- 1: 1
  id: 1
...

tarantool> require('net.box').connect('localhost', 3302).space.named:get(2):tomap()
---
- 1: 2
  2: 3
  nf: 3
  id: 2
...

> > +	assert(format != NULL);
> 
> Strictly speaking tuple_format_new() may fail with OOM.
> Please handle it.
>
Fixed.
 
> > +	tuple_format_ref(format);
> > +	lua_pushinteger(L, format->id);
> 
> Returning a cdata with gc set would look robuster to me - with an id
> it's pretty easy to leak an object.
> 
Fixed.

> > +	region_truncate(region, region_svp);
> > +	return 1;
> > +}
> > +
> > +static int
> > +netbox_format_delete(struct lua_State *L)
> > +{
> > +	int32_t format_id = luaL_checkinteger(L, 1);
> > +	if (format_id == box_tuple_format_default()->id)
> > +		return 0;
> > +	struct tuple_format *format = tuple_format_by_id(format_id);
> > +	assert(format != NULL);
> > +	tuple_format_unref(format);
> > +	return 0;
> > +}
> > +
> >  /**
> >   * Decode Tarantool response body consisting of single
> >   * IPROTO_DATA key into array of tuples.
> > @@ -619,6 +688,11 @@ netbox_decode_select(struct lua_State *L)
> >  {
> >  	uint32_t ctypeid;
> >  	const char *data = *(const char **)luaL_checkcdata(L, 1, &ctypeid);
> > +	uint32_t format_id;
> > +	if (lua_gettop(L) == 2)
> > +		format_id = luaL_checkinteger(L, 2);
> > +	else
> > +		format_id = box_tuple_format_default()->id;
> >  	assert(mp_typeof(*data) == MP_MAP);
> >  	uint32_t map_size = mp_decode_map(&data);
> >  	/* Until 2.0 body has no keys except DATA. */
> > @@ -627,7 +701,7 @@ netbox_decode_select(struct lua_State *L)
> >  	uint32_t key = mp_decode_uint(&data);
> >  	assert(key == IPROTO_DATA);
> >  	(void) key;
> > -	netbox_decode_data(L, &data);
> > +	netbox_decode_data(L, &data, format_id);
> >  	*(const char **)luaL_pushcdata(L, ctypeid) = data;
> >  	return 2;
> >  }
> > @@ -716,7 +790,8 @@ netbox_decode_execute(struct lua_State *L)
> >  		uint32_t key = mp_decode_uint(&data);
> >  		switch(key) {
> >  		case IPROTO_DATA:
> > -			netbox_decode_data(L, &data);
> > +			netbox_decode_data(L, &data,
> > +					   box_tuple_format_default()->id);
> >  			rows_index = i - map_size;
> >  			break;
> >  		case IPROTO_METADATA:
> > @@ -766,6 +841,8 @@ luaopen_net_box(struct lua_State *L)
> >  		{ "communicate",    netbox_communicate },
> >  		{ "decode_select",  netbox_decode_select },
> >  		{ "decode_execute", netbox_decode_execute },
> > +		{ "_format_new", netbox_format_new },
> > +		{ "_format_delete", netbox_format_delete },
> 
> I think we better place these helpers in src/box/lua/misc.cc - we might
> need them somewhere else. Also, this is 'internal' namespace so no need
> to prefix function names with an underscore.
> 
> Actually, I think we should add these functions in a separate patch and
> cover them with some basic tests involving argument checking (like
> passing an invalid field type or a field without a name).
> 
Done. Patch below.

> >  		{ NULL, NULL}
> >  	};
> >  	/* luaL_register_module polutes _G */
> > diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
> > index 8d42fb4..26ff7ff 100644
> > --- a/src/box/lua/net_box.lua
> > +++ b/src/box/lua/net_box.lua
> > @@ -63,12 +63,22 @@ local function decode_data(raw_data)
> >      local response, raw_end = decode(raw_data)
> >      return response[IPROTO_DATA_KEY], raw_end
> >  end
> > -local function decode_tuple(raw_data)
> > -    local response, raw_end = internal.decode_select(raw_data)
> > +local function decode_tuple(raw_data, raw_data_end, opts)
> 
> You use name 'args' when passing 'opts' to the encoder. Let's use name
> 'args' everywhere to avoid confusion.
> 
Fixed.

> > +    local response, raw_end
> > +    if opts ~= nil and opts.format_id ~= nil then
> > +        response, raw_end = internal.decode_select(raw_data, opts.format_id)
> > +    else
> > +        response, raw_end = internal.decode_select(raw_data)
> > +    end
> >      return response[1], raw_end
> >  end
> > -local function decode_get(raw_data)
> > -    local body, raw_end = internal.decode_select(raw_data)
> > +local function decode_get(raw_data, raw_data_end, opts)
> > +    local body, raw_end
> > +    if opts ~= nil and opts.format_id ~= nil then
> > +        body, raw_end = internal.decode_select(raw_data, opts.format_id)
> > +    else
> > +        body, raw_end = internal.decode_select(raw_data)
> > +    end
> 
> Can the 'else' branch be executed at all? Shouldn't 'format' always be
> available for decode_tuple and decode_get?
> 
You are right, fixed.

> >      if body[2] then
> >          return nil, raw_end, box.error.MORE_THAN_ONE_TUPLE
> >      end
> > @@ -82,6 +92,15 @@ local function decode_push(raw_data)
> >      local response, raw_end = decode(raw_data)
> >      return response[IPROTO_DATA_KEY][1], raw_end
> >  end
> > +local function decode_select(raw_data, raw_data_end, opts)
> > +    if opts ~= nil and opts.format_id ~= nil then
> > +        return internal.decode_select(raw_data, opts.format_id)
> > +    end
> > +    return internal.decode_select(raw_data)
> > +end
> > +local function decode_execute(raw_data, raw_data_end)
> > +    return internal.decode_execute(raw_data)
> > +end
> >  
> >  local function encode_call(send_buf, id, method_args)
> >      return internal.encode_call(send_buf, id, method_args.func_name,
> > @@ -157,7 +176,7 @@ local method_encoder = {
> >  
> >  local method_decoder = {
> >      ping    = decode_nil,
> > -    call_16 = internal.decode_select,
> > +    call_16 = decode_select,
> 
> I'd rather add decode_call_16 to avoid branching in decode_select
> (branching in a virtual method looks ugly IMO).
> 
Fixed.

> >      call_17 = decode_data,
> >      eval    = decode_data,
> >      insert  = decode_tuple,
> > @@ -165,8 +184,8 @@ local method_decoder = {
> >      delete  = decode_tuple,
> >      update  = decode_tuple,
> >      upsert  = decode_nil,
> > -    select  = internal.decode_select,
> > -    execute = internal.decode_execute,
> > +    select  = decode_select,
> > +    execute = decode_execute,
> >      get     = decode_get,
> >      min     = decode_get,
> >      max     = decode_get,
> > @@ -630,14 +649,15 @@ local function create_transport(host, port, user, password, callback,
> >          -- Decode xrow.body[DATA] to Lua objects
> >          if status == IPROTO_OK_KEY then
> >              request.response, real_end, request.errno =
> > -                method_decoder[request.method](body_rpos, body_end)
> > +                method_decoder[request.method](body_rpos, body_end,
> > +                                               request.method_args)
> >              assert(real_end == body_end, "invalid body length")
> >              requests[id] = nil
> >              request.id = nil
> >          else
> >              local msg
> >              msg, real_end, request.errno =
> > -                method_decoder.push(body_rpos, body_end)
> > +                method_decoder.push(body_rpos, body_end, request.method_args)
> 
> Looks unnecessary.
> 
Fixed.

> >              assert(real_end == body_end, "invalid body length")
> >              request.on_push(request.on_push_ctx, msg)
> >          end
> > @@ -1085,6 +1105,14 @@ end
> >  
> >  function remote_methods:close()
> >      check_remote_arg(self, 'close')
> > +    if (self.space ~= nil and type(self.space) == 'table') then
> > +        for _,space in pairs(self.space) do
> > +            if space.format_id ~= nil then
> > +                internal._format_delete(space._format_id)
> > +                space.format_id = nil
> > +            end
> > +        end
> > +    end
> >      self._transport.stop()
> >  end
> >  
> > @@ -1274,6 +1302,7 @@ function remote_methods:_install_schema(schema_version, spaces, indices,
> >          s.index = {}
> >          s.temporary = false
> >          s._format = format
> > +        s._format_id = internal._format_new(format)
> 
> Shouldn't you unref the old format before setting the new one?
> 
Fixed since __gc is set now.

> > +-- gh-2978: field names for tuples received from netbox.
> > +--
> > +_ = box.schema.create_space("named", {format = {{name = "id"}, {name="abc"}}})
> > +---
> > +...
> > +_ = box.space.named:create_index('id', {parts = {{1, 'unsigned'}}})
> > +---
> > +...
> > +box.space.named:insert({1, 1})
> > +---
> > +- [1, 1]
> > +...
> > +box.schema.user.grant('guest', 'read, write, execute', 'universe')
> 
> Please don't use 'universe' in tests - we're trying to deprecate it
> AFAIR. Grant specific permissions instead.
> 
Fixed.

> It would be nice to check that schema reload does update the format,
> i.e. update the space format, call remote methods again and check
> that they do use new names.
> 
Done.


First new patch:

>From 63075deb8411643d4af0cf27be2c024b5a20222c Mon Sep 17 00:00:00 2001
Date: Tue, 11 Jun 2019 15:21:39 +0300
Subject: [PATCH] lua: internal function to parse space format

This patch defines a new function that parses space format and
returns it to Lua as cdata. For this cdata destructor is included.

Needed for #2978

diff --git a/src/box/lua/misc.cc b/src/box/lua/misc.cc
index 8de7401..e62e2ba 100644
--- a/src/box/lua/misc.cc
+++ b/src/box/lua/misc.cc
@@ -37,9 +37,13 @@
 
 #include "box/box.h"
 #include "box/port.h"
+#include "box/tuple.h"
+#include "box/tuple_format.h"
 #include "box/lua/tuple.h"
 #include "mpstream.h"
 
+uint32_t CTID_STRUCT_TUPLE_FORMAT_PTR;
+
 /** {{{ Miscellaneous utils **/
 
 char *
@@ -116,14 +120,117 @@ lbox_select(lua_State *L)
 
 /* }}} */
 
+static int
+lbox_tuple_format_gc(struct lua_State *L)
+{
+	uint32_t ctypeid;
+	struct tuple_format *format =
+		*(struct tuple_format **)luaL_checkcdata(L, 1, &ctypeid);
+	if (ctypeid != CTID_STRUCT_TUPLE_FORMAT_PTR) {
+		luaL_error(L, "Invalid argument: 'struct tuple_format *' "\
+			   "expected, got %s)",
+			   lua_typename(L, lua_type(L, 1)));
+	}
+	box_tuple_format_unref(format);
+	return 0;
+}
+
+static int
+lbox_push_tuple_format(struct lua_State *L, struct tuple_format *format)
+{
+	struct tuple_format **ptr = (struct tuple_format **)
+		luaL_pushcdata(L, CTID_STRUCT_TUPLE_FORMAT_PTR);
+	*ptr = format;
+	box_tuple_format_ref(format);
+	lua_pushcfunction(L, lbox_tuple_format_gc);
+	luaL_setcdatagc(L, -2);
+	return 1;
+}
+
+static int
+lbox_format_new(struct lua_State *L)
+{
+	assert(CTID_STRUCT_TUPLE_FORMAT_PTR != 0);
+	if (lua_gettop(L) != 1 || ! lua_istable(L, 1))
+		return luaL_error(L, "Usage: box.internal.format_new(format)");
+	uint32_t count = lua_objlen(L, 1);
+	if (count == 0)
+		return lbox_push_tuple_format(L, tuple_format_runtime);
+	size_t size = count * sizeof(struct field_def);
+	struct region *region = &fiber()->gc;
+	size_t region_svp = region_used(region);
+	struct field_def *fields =
+		(struct field_def *)region_alloc(region, size);
+	if (fields == NULL) {
+		diag_set(OutOfMemory, size, "region_alloc", "fields");
+		return luaT_error(L);
+	}
+	for (uint32_t i = 0; i < count; ++i) {
+		size_t len;
+
+		fields[i] = field_def_default;
+
+		lua_pushinteger(L, i + 1);
+		lua_gettable(L, 1);
+
+		lua_pushstring(L, "type");
+		lua_gettable(L, -2);
+		if (! lua_isnil(L, -1)) {
+			const char *type_name = lua_tolstring(L, -1, &len);
+			fields[i].type = field_type_by_name(type_name, len);
+			if (fields[i].type == field_type_MAX) {
+				const char *err =
+					"field %d has unknown field type";
+				return luaL_error(L, tt_sprintf(err, i + 1));
+			}
+		}
+		lua_pop(L, 1);
+
+		lua_pushstring(L, "name");
+		lua_gettable(L, -2);
+		if (lua_isnil(L, -1)) {
+			return luaL_error(L, tt_sprintf("field %d name is not "\
+							"specified", i + 1));
+		}
+		const char *name = lua_tolstring(L, -1, &len);
+		fields[i].name = (char *)region_alloc(region, len + 1);
+		if (fields == NULL) {
+			diag_set(OutOfMemory, size, "region_alloc",
+				 "fields[i].name");
+			return luaT_error(L);
+		}
+		memcpy(fields[i].name, name, len);
+		fields[i].name[len] = '\0';
+		lua_pop(L, 1);
+		lua_pop(L, 1);
+	}
+	struct tuple_dictionary *dict = tuple_dictionary_new(fields, count);
+	region_truncate(region, region_svp);
+	if (dict == NULL)
+		return luaT_error(L);
+	struct tuple_format *format =
+		tuple_format_new(&tuple_format_runtime->vtab, NULL, NULL, 0,
+				 NULL, 0, 0, dict, false, false);
+	if (format == NULL)
+		return luaT_error(L);
+	return lbox_push_tuple_format(L, format);
+}
+
 void
 box_lua_misc_init(struct lua_State *L)
 {
 	static const struct luaL_Reg boxlib_internal[] = {
 		{"select", lbox_select},
+		{"format_new", lbox_format_new},
 		{NULL, NULL}
 	};
 
 	luaL_register(L, "box.internal", boxlib_internal);
 	lua_pop(L, 1);
+
+	int rc = luaL_cdef(L, "struct tuple_format;");
+	assert(rc == 0);
+	(void) rc;
+	CTID_STRUCT_TUPLE_FORMAT_PTR = luaL_ctypeid(L, "struct tuple_format *");
+	assert(CTID_STRUCT_TUPLE_FORMAT_PTR != 0);
 }
diff --git a/test/box/misc.result b/test/box/misc.result
index 43b5a4a..ce39bbb 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -1271,3 +1271,51 @@ box.cfg{too_long_threshold = too_long_threshold}
 s:drop()
 ---
 ...
+--
+-- gh-2978: Function to parse space format.
+--
+format = {}
+---
+...
+format[1] = {}
+---
+...
+format[1].type = 'unsigned'
+---
+...
+box.internal.format_new(format)
+---
+- error: field 1 name is not specified
+...
+format[1].name = 'aaa'
+---
+...
+format[2] = {}
+---
+...
+format[2].name = 'aaa'
+---
+...
+format[2].type = 'any'
+---
+...
+box.internal.format_new(format)
+---
+- error: Space field 'aaa' is duplicate
+...
+format[2].name = 'bbb'
+---
+...
+format[3] = {}
+---
+...
+format[3].name = 'ccc'
+---
+...
+format[3].type = 'something'
+---
+...
+box.internal.format_new(format)
+---
+- error: field 3 has unknown field type
+...
diff --git a/test/box/misc.test.lua b/test/box/misc.test.lua
index 75d937e..2d13b99 100644
--- a/test/box/misc.test.lua
+++ b/test/box/misc.test.lua
@@ -353,3 +353,23 @@ lsn == expected_lsn
 box.cfg{too_long_threshold = too_long_threshold}
 s:drop()
 
+
+--
+-- gh-2978: Function to parse space format.
+--
+format = {}
+format[1] = {}
+format[1].type = 'unsigned'
+box.internal.format_new(format)
+
+format[1].name = 'aaa'
+format[2] = {}
+format[2].name = 'aaa'
+format[2].type = 'any'
+box.internal.format_new(format)
+
+format[2].name = 'bbb'
+format[3] = {}
+format[3].name = 'ccc'
+format[3].type = 'something'
+box.internal.format_new(format)




Second new patch:

>From f9e959e3f0f38e8f6b8f1294ba29c28d41f30968 Mon Sep 17 00:00:00 2001
Date: Tue, 11 Jun 2019 16:36:39 +0300
Subject: [PATCH] netbox: define formats for tuple from netbox

This patch creates tuple_formats for the tuples obtained through
the netbox.

Closes #2978

@TarantoolBot document
Title: Field names for tuples received from net.box

It is possible now to access by field name for tuples received
from net.box. For example:

box.cfg{listen = 3302}
box.schema.user.grant('guest','read, write, execute', 'space')
box.schema.user.grant('guest', 'create', 'space')

box.schema.create_space("named", {format = {{name = "id"}}})
box.space.named:create_index('id', {parts = {{1, 'unsigned'}}})
box.space.named:insert({1})
require('net.box').connect('localhost', 3302).space.named:get(1).id

Result:

tarantool> require('net.box').connect('localhost', 3302).space.named:get(1).id
---
- 1
...

diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
index 7484a86..946d397 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -590,12 +590,11 @@ netbox_encode_execute(lua_State *L)
  * @param data MessagePack.
  */
 static void
-netbox_decode_data(struct lua_State *L, const char **data)
+netbox_decode_data(struct lua_State *L, const char **data,
+		   struct tuple_format *format)
 {
 	uint32_t count = mp_decode_array(data);
 	lua_createtable(L, count, 0);
-	struct tuple_format *format =
-		box_tuple_format_default();
 	for (uint32_t j = 0; j < count; ++j) {
 		const char *begin = *data;
 		mp_next(data);
@@ -618,6 +617,15 @@ static int
 netbox_decode_select(struct lua_State *L)
 {
 	uint32_t ctypeid;
+	int top = lua_gettop(L);
+	assert(top == 1 || top == 2);
+	struct tuple_format *format;
+	if (top == 2 && lua_type(L, 2) == LUA_TCDATA) {
+		format = *(struct tuple_format **)luaL_checkcdata(L, 2,
+								  &ctypeid);
+	} else {
+		format = tuple_format_runtime;
+	}
 	const char *data = *(const char **)luaL_checkcdata(L, 1, &ctypeid);
 	assert(mp_typeof(*data) == MP_MAP);
 	uint32_t map_size = mp_decode_map(&data);
@@ -627,7 +635,7 @@ netbox_decode_select(struct lua_State *L)
 	uint32_t key = mp_decode_uint(&data);
 	assert(key == IPROTO_DATA);
 	(void) key;
-	netbox_decode_data(L, &data);
+	netbox_decode_data(L, &data, format);
 	*(const char **)luaL_pushcdata(L, ctypeid) = data;
 	return 2;
 }
@@ -716,7 +724,7 @@ netbox_decode_execute(struct lua_State *L)
 		uint32_t key = mp_decode_uint(&data);
 		switch(key) {
 		case IPROTO_DATA:
-			netbox_decode_data(L, &data);
+			netbox_decode_data(L, &data, tuple_format_runtime);
 			rows_index = i - map_size;
 			break;
 		case IPROTO_METADATA:
diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index d9838f8..30cf227 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -63,12 +63,12 @@ local function decode_data(raw_data)
     local response, raw_end = decode(raw_data)
     return response[IPROTO_DATA_KEY], raw_end
 end
-local function decode_tuple(raw_data)
-    local response, raw_end = internal.decode_select(raw_data)
+local function decode_tuple(raw_data, raw_data_end, args)
+    local response, raw_end = internal.decode_select(raw_data, args.format)
     return response[1], raw_end
 end
-local function decode_get(raw_data)
-    local body, raw_end = internal.decode_select(raw_data)
+local function decode_get(raw_data, raw_data_end, args)
+    local body, raw_end = internal.decode_select(raw_data, args.format)
     if body[2] then
         return nil, raw_end, box.error.MORE_THAN_ONE_TUPLE
     end
@@ -82,6 +82,12 @@ local function decode_push(raw_data)
     local response, raw_end = decode(raw_data)
     return response[IPROTO_DATA_KEY][1], raw_end
 end
+local function decode_call_16(raw_data)
+    return internal.decode_select(raw_data)
+end
+local function decode_select(raw_data, raw_data_end, args)
+    return internal.decode_select(raw_data, args.format)
+end
 
 local function encode_call(send_buf, id, args)
     return internal.encode_call(send_buf, id, args.func_name, args.args)
@@ -150,7 +156,7 @@ local method_encoder = {
 
 local method_decoder = {
     ping    = decode_nil,
-    call_16 = internal.decode_select,
+    call_16 = decode_call_16,
     call_17 = decode_data,
     eval    = decode_data,
     insert  = decode_tuple,
@@ -158,7 +164,7 @@ local method_decoder = {
     delete  = decode_tuple,
     update  = decode_tuple,
     upsert  = decode_nil,
-    select  = internal.decode_select,
+    select  = decode_select,
     execute = internal.decode_execute,
     get     = decode_get,
     min     = decode_get,
@@ -623,7 +629,8 @@ local function create_transport(host, port, user, password, callback,
         -- Decode xrow.body[DATA] to Lua objects
         if status == IPROTO_OK_KEY then
             request.response, real_end, request.errno =
-                method_decoder[request.method](body_rpos, body_end)
+                method_decoder[request.method](body_rpos, body_end,
+                                               request.args)
             assert(real_end == body_end, "invalid body length")
             requests[id] = nil
             request.id = nil
@@ -1267,6 +1274,7 @@ function remote_methods:_install_schema(schema_version, spaces, indices,
         s.index = {}
         s.temporary = false
         s._format = format
+        s._format_cdata = box.internal.format_new(format)
         s.connection = self
         if #space > 5 then
             local opts = space[6]
@@ -1384,13 +1392,15 @@ space_metatable = function(remote)
 
     function methods:insert(tuple, opts)
         check_space_arg(self, 'insert')
-        local args = {space_id = self.id, tuple = tuple}
+        local args = {space_id = self.id, tuple = tuple,
+                      format = self._format_cdata}
         return remote:_request('insert', opts, args)
     end
 
     function methods:replace(tuple, opts)
         check_space_arg(self, 'replace')
-        local args = {space_id = self.id, tuple = tuple}
+        local args = {space_id = self.id, tuple = tuple,
+                      format = self._format_cdata}
         return remote:_request('replace', opts, args)
     end
 
@@ -1443,7 +1453,7 @@ index_metatable = function(remote)
         local limit = tonumber(opts and opts.limit) or 0xFFFFFFFF
         local args = {space_id = self.space.id, index_id = self.id,
                       iterator = iterator, offset = offset, limit = limit,
-                      key = key}
+                      key = key, format = self.space._format_cdata}
         return (remote:_request('select', opts, args))
     end
 
@@ -1453,7 +1463,8 @@ index_metatable = function(remote)
             error("index:get() doesn't support `buffer` argument")
         end
         local args = {space_id = self.space.id, index_id = self.id,
-                      iterator = box.index.EQ, offset = 0, limit = 2, key = key}
+                      iterator = box.index.EQ, offset = 0, limit = 2, key = key,
+                      format = self.space._format_cdata}
         return nothing_or_data(remote:_request('get', opts, args))
     end
 
@@ -1463,7 +1474,8 @@ index_metatable = function(remote)
             error("index:min() doesn't support `buffer` argument")
         end
         local args = {space_id = self.space.id, index_id = self.id,
-                      iterator = box.index.GE, offset = 0, limit = 1, key = key}
+                      iterator = box.index.GE, offset = 0, limit = 1, key = key,
+                      format = self.space._format_cdata}
         return nothing_or_data(remote:_request('min', opts, args))
     end
 
@@ -1473,7 +1485,8 @@ index_metatable = function(remote)
             error("index:max() doesn't support `buffer` argument")
         end
         local args = {space_id = self.space.id, index_id = self.id,
-                      iterator = box.index.LE, offset = 0, limit = 1, key = key}
+                      iterator = box.index.LE, offset = 0, limit = 1, key = key,
+                      format = self.space._format_cdata}
         return nothing_or_data(remote:_request('max', opts, args))
     end
 
@@ -1490,14 +1503,15 @@ index_metatable = function(remote)
 
     function methods:delete(key, opts)
         check_index_arg(self, 'delete')
-        local args = {space_id = self.space.id, index_id = self.id, key = key}
+        local args = {space_id = self.space.id, index_id = self.id, key = key,
+                      format = self.space._format_cdata}
         return nothing_or_data(remote:_request('delete', opts, args))
     end
 
     function methods:update(key, oplist, opts)
         check_index_arg(self, 'update')
         local args = {space_id = self.space.id, index_id = self.id, key = key,
-                      oplist = oplist}
+                      oplist = oplist, format = self.space._format_cdata}
         return nothing_or_data(remote:_request('update', opts, args))
     end
 
diff --git a/test/box/net.box.result b/test/box/net.box.result
index e87a60b..413bdec 100644
--- a/test/box/net.box.result
+++ b/test/box/net.box.result
@@ -3572,6 +3572,104 @@ box.schema.func.drop('change_schema')
 ---
 ...
 --
+-- gh-2978: field names for tuples received from netbox.
+--
+_ = box.schema.create_space("named", {format = {{name = "id"}, {name="abc"}}})
+---
+...
+_ = box.space.named:create_index('id', {parts = {{1, 'unsigned'}}})
+---
+...
+box.space.named:insert({1, 1})
+---
+- [1, 1]
+...
+box.schema.user.grant('guest', 'read, write, execute', 'space')
+---
+...
+cn = net.connect(box.cfg.listen)
+---
+...
+s = cn.space.named
+---
+...
+s:get{1}.id
+---
+- 1
+...
+s:get{1}:tomap()
+---
+- 1: 1
+  2: 1
+  abc: 1
+  id: 1
+...
+s:insert{2,3}:tomap()
+---
+- 1: 2
+  2: 3
+  abc: 3
+  id: 2
+...
+s:replace{2,14}:tomap()
+---
+- 1: 2
+  2: 14
+  abc: 14
+  id: 2
+...
+s:update(1, {{'+', 2, 10}}):tomap()
+---
+- 1: 1
+  2: 11
+  abc: 11
+  id: 1
+...
+s:select()[1]:tomap()
+---
+- 1: 1
+  2: 11
+  abc: 11
+  id: 1
+...
+s:delete({2}):tomap()
+---
+- 1: 2
+  2: 14
+  abc: 14
+  id: 2
+...
+-- Check that formats changes after reload.
+box.space.named:format({{name = "id2"}, {name="abc2"}})
+---
+...
+s:select()[1]:tomap()
+---
+- 1: 1
+  2: 11
+  abc: 11
+  id: 1
+...
+cn:reload_schema()
+---
+...
+s:select()[1]:tomap()
+---
+- 1: 1
+  2: 11
+  id2: 1
+  abc2: 11
+...
+cn:close()
+---
+...
+box.space.named:drop()
+---
+...
+box.schema.user.revoke('guest', 'read, write, execute', 'space')
+---
+...
+--
 -- gh-3400: long-poll input discard must not touch event loop of
 -- a closed connection.
 --
diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
index 6e58da8..759cc4f 100644
--- a/test/box/net.box.test.lua
+++ b/test/box/net.box.test.lua
@@ -1433,6 +1433,34 @@ box.space.test3:drop()
 box.schema.func.drop('change_schema')
 
 --
+-- gh-2978: field names for tuples received from netbox.
+--
+_ = box.schema.create_space("named", {format = {{name = "id"}, {name="abc"}}})
+_ = box.space.named:create_index('id', {parts = {{1, 'unsigned'}}})
+box.space.named:insert({1, 1})
+box.schema.user.grant('guest', 'read, write, execute', 'space')
+cn = net.connect(box.cfg.listen)
+
+s = cn.space.named
+s:get{1}.id
+s:get{1}:tomap()
+s:insert{2,3}:tomap()
+s:replace{2,14}:tomap()
+s:update(1, {{'+', 2, 10}}):tomap()
+s:select()[1]:tomap()
+s:delete({2}):tomap()
+
+-- Check that formats changes after reload.
+box.space.named:format({{name = "id2"}, {name="abc2"}})
+s:select()[1]:tomap()
+cn:reload_schema()
+s:select()[1]:tomap()
+
+cn:close()
+box.space.named:drop()
+box.schema.user.revoke('guest', 'read, write, execute', 'space')
+
+--
 -- gh-3400: long-poll input discard must not touch event loop of
 -- a closed connection.
 --




More information about the Tarantool-patches mailing list