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 ED6622DD63 for ; Fri, 6 Apr 2018 11:44:05 -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 mdV6EdmmANez for ; Fri, 6 Apr 2018 11:44:05 -0400 (EDT) Received: from smtp58.i.mail.ru (smtp58.i.mail.ru [217.69.128.38]) (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 06C1F2DD61 for ; Fri, 6 Apr 2018 11:44:04 -0400 (EDT) Date: Fri, 6 Apr 2018 18:44:01 +0300 From: Konstantin Osipov Subject: [tarantool-patches] Re: [PATCH v2 1/1] schema:frommap() to create table or tuple Message-ID: <20180406154401.GA18456@atlas> References: <832dae8c-3736-1547-b0a4-75f1c8debc52@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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: tarantool-patches@freelists.org Cc: v.shpilevoy@tarantool.org, Kirill Shcherbatov * Kirill Shcherbatov [18/04/05 10:24]: > This feature is designed to build tuple or table object by map. > Tuple output is default. If you wish to make a table, specify table = true option. > subscriber:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {table = true}) What are the performance results from adding the cache? How much do we win? It's OK to export _schema_version to Lua as extern int via FFI, and avoid a function call. > +static int > +lbox_space_ptr_cached(struct lua_State *L) The fast path of this function should be in plain Lua. You have to re-fetch the space via lookup in a slowpath only, when schema version has changed. What about using the cptr in all other functions? > +{ > + if (lua_gettop(L) < 1 || !lua_istable(L, 1)) > + luaL_error(L, "Usage: space:_cptr()"); > + > + lua_getfield(L, 1, "_schema_version"); > + uint64_t schema_version = luaL_touint64(L, -1); > + lua_pop(L, 1); > + uint64_t global_shema_version = box_schema_version(); > + > + void *space = NULL; > + if (schema_version == global_shema_version) { > + lua_getfield(L, 1, "_cptr"); > + space = (void *)lua_topointer(L, -1); > + } else { > + lua_getfield(L, 1, "id"); > + uint32_t id = luaL_touint64(L, -1); > + space = space_by_id(id); > + if (space == NULL) { > + lua_pushnil(L); > + lua_pushstring(L, tt_sprintf("Space with id '%d' "\ > + "doesn't exist", id)); > + return 2; > + } > + > + /** update the cache */ > + luaL_pushuint64(L, global_shema_version); > + lua_setfield(L, 1, "_schema_version"); > + lua_pushlightuserdata(L, space); > + lua_setfield(L, 1, "_cptr"); > + } > + lua_pop(L, 1); > + lua_pushlightuserdata(L, space); > + return 1; > +} > + > +/** > + * Make a tuple or a table Lua object by map. > + * @param Lua space object. > + * @param Lua map table object. > + * @param Lua opts table object (optional). > + * @retval not nil A tuple or a table conforming to a space > + * format. > + * @retval nil, err Can not built a tuple. A reason is returned in > + * the second value. > + */ > +static int > +lbox_space_frommap(struct lua_State *L) > +{ > + struct tuple_dictionary *dict = NULL; > + struct space *space = NULL; > + int rc, argc = lua_gettop(L); > + bool table = false; > + if (argc < 2 || argc > 3 || !lua_istable(L, 2)) > + goto usage_error; > + if (argc == 3) { > + if (!lua_istable(L, 3)) > + goto usage_error; > + lua_getfield(L, 3, "table"); > + if (!lua_isboolean(L, -1) && !lua_isnil(L, -1)) > + goto usage_error; > + table = lua_toboolean(L, -1); > + } > + > + rc = lbox_space_ptr_cached(L); > + if (rc != 1) > + return rc; > + space = (struct space *) lua_topointer(L, -1); > + dict = space->format->dict; > + lua_createtable(L, space->def->field_count, 0); > + > + lua_pushnil(L); > + while (lua_next(L, 2) != 0) { > + uint32_t fieldno; > + size_t key_len; > + const char *key = lua_tolstring(L, -2, &key_len); > + uint32_t key_hash = lua_hashstring(L, -2); > + if (tuple_fieldno_by_name(dict, key, key_len, key_hash, > + &fieldno)) { > + lua_pushnil(L); > + lua_pushstring(L, tt_sprintf("Unknown field '%s'", > + key)); > + return 2; > + } > + lua_rawseti(L, -3, fieldno+1); > + } > + if (table) > + return 1; > + > + lua_replace(L, 1); > + lua_settop(L, 1); > + return lbox_tuple_new(L); > +usage_error: > + return luaL_error(L, "Usage: space:frommap(map, opts)"); > +} > + > void > box_lua_space_init(struct lua_State *L) > { > @@ -464,4 +579,12 @@ box_lua_space_init(struct lua_State *L) > lua_pushnumber(L, VCLOCK_MAX); > lua_setfield(L, -2, "REPLICA_MAX"); > lua_pop(L, 2); /* box, schema */ > + > + static const struct luaL_Reg space_internal_lib[] = { > + {"frommap", lbox_space_frommap}, > + {"_cptr", lbox_space_ptr_cached}, > + {NULL, NULL} > + }; > + luaL_register(L, "box.internal.space", space_internal_lib); > + lua_pop(L, 1); > } > diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c > index 7ca4299..92a7185 100644 > --- a/src/box/lua/tuple.c > +++ b/src/box/lua/tuple.c > @@ -90,7 +90,7 @@ luaT_istuple(struct lua_State *L, int narg) > return *(struct tuple **) data; > } > > -static int > +int > lbox_tuple_new(lua_State *L) > { > int argc = lua_gettop(L); > diff --git a/src/box/lua/tuple.h b/src/box/lua/tuple.h > index 0965644..5d7062e 100644 > --- a/src/box/lua/tuple.h > +++ b/src/box/lua/tuple.h > @@ -66,6 +66,9 @@ luaT_istuple(struct lua_State *L, int idx); > > /** \endcond public */ > > +int > +lbox_tuple_new(struct lua_State *L); > + > static inline int > luaT_pushtupleornil(struct lua_State *L, struct tuple *tuple) > { > diff --git a/test/box/tuple.result b/test/box/tuple.result > index ec5ed90..2dd8f0d 100644 > --- a/test/box/tuple.result > +++ b/test/box/tuple.result > @@ -1060,6 +1060,101 @@ box.tuple.bsize == t.bsize > --- > - true > ... > +-- > +-- gh-3282: space:frommap(). > +-- > +format = {} > +--- > +... > +format[1] = {name = 'aaa', type = 'unsigned'} > +--- > +... > +format[2] = {name = 'bbb', type = 'unsigned'} > +--- > +... > +format[3] = {name = 'ccc', type = 'unsigned'} > +--- > +... > +format[4] = {name = 'ddd', type = 'unsigned'} > +--- > +... > +s = box.schema.create_space('test', {format = format}) > +--- > +... > +s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}) > +--- > +- [2, 4, 3, 1] > +... > +s:frommap({ddd = 1, aaa = 2, bbb = 3}) > +--- > +- [2, 3, null, 1] > +... > +s:frommap({ddd = 1, aaa = 2, ccc = 3, eee = 4}) > +--- > +- null > +- Unknown field 'eee' > +... > +s:frommap() > +--- > +- error: 'Usage: space:frommap(map, opts)' > +... > +s:frommap({}) > +--- > +- [] > +... > +s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {table = true}) > +--- > +- - 2 > + - 4 > + - 3 > + - 1 > +... > +s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {table = false}) > +--- > +- [2, 4, 3, 1] > +... > +s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = box.NULL}) > +--- > +- [2, null, 3, 1] > +... > +s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {dummy = true}) > +--- > +- [2, 4, 3, 1] > +... > +_ = s:create_index('primary', {parts = {'aaa'}}) > +--- > +... > +tuple = s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}) > +--- > +... > +s:replace(tuple) > +--- > +- [2, 4, 3, 1] > +... > +_, err = box.internal.space._cptr(s) > +--- > +... > +assert(not err) > +--- > +- true > +... > +s:drop() > +--- > +... > +_, err = s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}) > +--- > +... > +assert(err ~= nil) > +--- > +- true > +... > +_, err = box.internal.space._cptr(s) > +--- > +... > +assert(err ~= nil) > +--- > +- true > +... > test_run:cmd("clear filter") > --- > - true > diff --git a/test/box/tuple.test.lua b/test/box/tuple.test.lua > index 0b3392d..03e6194 100644 > --- a/test/box/tuple.test.lua > +++ b/test/box/tuple.test.lua > @@ -349,4 +349,33 @@ box.tuple.update == t.update > box.tuple.upsert == t.upsert > box.tuple.bsize == t.bsize > > +-- > +-- gh-3282: space:frommap(). > +-- > +format = {} > +format[1] = {name = 'aaa', type = 'unsigned'} > +format[2] = {name = 'bbb', type = 'unsigned'} > +format[3] = {name = 'ccc', type = 'unsigned'} > +format[4] = {name = 'ddd', type = 'unsigned'} > +s = box.schema.create_space('test', {format = format}) > +s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}) > +s:frommap({ddd = 1, aaa = 2, bbb = 3}) > +s:frommap({ddd = 1, aaa = 2, ccc = 3, eee = 4}) > +s:frommap() > +s:frommap({}) > +s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {table = true}) > +s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {table = false}) > +s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = box.NULL}) > +s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}, {dummy = true}) > +_ = s:create_index('primary', {parts = {'aaa'}}) > +tuple = s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}) > +s:replace(tuple) > +_, err = box.internal.space._cptr(s) > +assert(not err) > +s:drop() > +_, err = s:frommap({ddd = 1, aaa = 2, ccc = 3, bbb = 4}) > +assert(err ~= nil) > +_, err = box.internal.space._cptr(s) > +assert(err ~= nil) > + > test_run:cmd("clear filter") > -- > 2.7.4 > > -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov