[PATCH v1 2/2] netbox: define formats for tuple from netbox
Vladimir Davydov
vdavydov.dev at gmail.com
Tue Jun 18 11:55:53 MSK 2019
On Fri, Jun 14, 2019 at 03:29:21PM +0300, Mergen Imeev wrote:
> First new patch:
It looks like it's time to send v2 in a separate thread.
See a few minor comments inline.
>
> 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;
> +
Should be static.
> /** {{{ 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 *' "\
Backslash (\) is not necessary.
Anyway, I think we better factor this check out into a separate
function, like lboxk_check_tuple_format, so that we could easily
reuse it in case we introduce some new functions taking a format.
> + "expected, got %s)",
> + lua_typename(L, lua_type(L, 1)));
> + }
> + box_tuple_format_unref(format);
Please use tuple_format_ref/tuple_format_unref.
box_* functions exist for external modules. No need to use them here.
> + 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)
Please be consistent and use tuple_format prefix everywhere.
> +{
> + 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)");
I would rather name this function box.internal.new_tuple_format.
Also, I think we should allow calling it without any arguments, in which
case it should be equivalent to box.internal.new_tuple_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 "\
Useless backslash (\).
> + "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
> +...
Please write more tests:
- setting 'format' without 'type'
- passing something different from a table to the function
- passing the value returned by space:format() to this function
- setting is_nullable in the input table
More information about the Tarantool-patches
mailing list