From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 1BAA128A12 for ; Wed, 22 Aug 2018 10:16:08 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 6kP5VVWMT6zI for ; Wed, 22 Aug 2018 10:16:08 -0400 (EDT) Received: from smtp34.i.mail.ru (smtp34.i.mail.ru [94.100.177.94]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id CB85A289B7 for ; Wed, 22 Aug 2018 10:16:07 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v1 1/1] lua: wrong 'tomap' work with nullable fields References: <7d7cd9396f57d25a6bd05a24bf8677e7bb0fdd28.1534934562.git.imeevma@gmail.com> From: Vladislav Shpilevoy Message-ID: <90fa7a3e-d737-ea85-d4e4-0a0b0ef376d0@tarantool.org> Date: Wed, 22 Aug 2018 17:16:05 +0300 MIME-Version: 1.0 In-Reply-To: <7d7cd9396f57d25a6bd05a24bf8677e7bb0fdd28.1534934562.git.imeevma@gmail.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: imeevma@tarantool.org, tarantool-patches@freelists.org Hi! Thanks for the patch! See 3 comments below. On 22/08/2018 13:43, imeevma@tarantool.org wrote: > Tuple method 'tomap' in some cases worked impropertly if tuple 1. Typo: 'impropertly'. > length less than it should be according to space format. Fixed > in this patch. > > Closes #3631 > --- > Branch: https://github.com/tarantool/tarantool/tree/imeevma/gh-3631-wrong-tomap-work-with-nullable-fields > Issue: https://github.com/tarantool/tarantool/issues/3631 > > src/box/lua/tuple.c | 4 +-- > test/engine/tuple.result | 63 ++++++++++++++++++++++++++++++++++++++++++++++ > test/engine/tuple.test.lua | 23 +++++++++++++++++ > 3 files changed, 88 insertions(+), 2 deletions(-) > > diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c > index 7ca4299..856042c 100644 > --- a/src/box/lua/tuple.c > +++ b/src/box/lua/tuple.c > @@ -284,12 +284,12 @@ lbox_tuple_to_map(struct lua_State *L) > luaL_error(L, "Usage: tuple:tomap()"); > const struct tuple *tuple = lua_checktuple(L, 1); > const struct tuple_format *format = tuple_format(tuple); > - const struct tuple_field *field = &format->fields[0]; > const char *pos = tuple_data(tuple); > int field_count = (int)mp_decode_array(&pos); > int n_named = format->dict->name_count; > lua_createtable(L, field_count, n_named); > - for (int i = 0; i < n_named; ++i, ++field) { > + int fields_number = n_named > field_count ? field_count : n_named; 2. Please, use MIN macros. It is available in this file as I remember. > + for (int i = 0; i < fields_number; ++i) { > /* Access by name. */ > const char *name = format->dict->names[i]; > lua_pushstring(L, name); > diff --git a/test/engine/tuple.result b/test/engine/tuple.result > index b3b23b2..b9f42b9 100644 > --- a/test/engine/tuple.result > +++ b/test/engine/tuple.result > @@ -590,6 +590,69 @@ maplen(t1map), t1map[1], t1map[2], t1map[3] > s:drop() > --- > ... > +-- > +-- gh-3631: Wrong 'tomap' work with nullable fields > +-- > +format = {} > +--- > +... > +format[1] = {'first', 'unsigned'} > +--- > +... > +format[2] = {'second', 'unsigned'} > +--- > +... > +format[3] = {'third', 'unsigned'} > +--- > +... > +format[4] = {'fourth', 'string', is_nullable = true} > +--- > +... > +s = box.schema.space.create('test', {format = format, engine = engine}) > +--- > +... > +pk = s:create_index('primary', {parts = { 1, 'unsigned'}}) 3. Odd white space before '1'. > +--- > +... > +s:insert({1, 2, 3}) > +--- > +- [1, 2, 3] > +... > +tuple = s:get(1) > +--- > +...